diff --git a/.trellis/spec/backend/error-handling.md b/.trellis/spec/backend/error-handling.md index 72f94e0..fa5a9d4 100644 --- a/.trellis/spec/backend/error-handling.md +++ b/.trellis/spec/backend/error-handling.md @@ -33,6 +33,8 @@ subprocess.run(f"nssm.exe start {service_name}", shell=True) `nssm.exe` 找不到或返回非零退出码时，CLI 命令失败，并把 stderr 或 stdout 中的系统错误摘要展示给用户。 +捕获 `nssm.exe` stdout/stderr 时必须显式指定 `encoding="utf-8"` 和 `errors="replace"`，不得依赖 Windows 系统默认编码。外部输出中出现无法按 UTF-8 解码的字节时，保留替换字符并继续走正常的返回码与错误摘要处理。 + ## 服务状态错误 - `sing start ` 在服务已运行时失败，并提示使用 `sing restart `。 diff --git a/.trellis/spec/backend/quality-guidelines.md b/.trellis/spec/backend/quality-guidelines.md index 49d3fd6..b677f89 100644 --- a/.trellis/spec/backend/quality-guidelines.md +++ b/.trellis/spec/backend/quality-guidelines.md @@ -63,6 +63,7 @@ nssm.exe remove sing-box confirm - `sing restart ` 必须先停止服务，停止成功后再写入 NSSM 参数并启动服务。 - `nssm.exe` 必须从 `PATH` 解析；项目不下载、不内置 NSSM。 - 外部命令必须用参数列表调用，不通过 shell 字符串执行。 +- 捕获外部命令输出时必须显式使用 `encoding="utf-8"` 和 `errors="replace"`，避免 Windows locale 默认编码导致 reader thread 抛出 `UnicodeDecodeError`。 ### 4. Validation & Error Matrix @@ -70,12 +71,13 @@ nssm.exe remove sing-box confirm |------|------| | `nssm.exe` 不在 `PATH` | 命令失败并提示安装 NSSM 且让 `nssm.exe` 可从 `PATH` 访问 | | `nssm.exe` 返回非零退出码 | 命令失败并暴露 stderr 或 stdout 摘要 | +| `nssm.exe` 输出包含当前系统编码无法解码的字节 | 命令不因解码崩溃，输出中的非法字节以替换字符呈现 | | `nssm.exe status sing-box` 输出 `SERVICE_RUNNING` | `service_is_running()` 返回 `True` | | `sing start ` 发现服务已运行 | 命令失败并提示使用 `sing restart ` | ### 5. Good/Base/Bad Cases -- Good: `subprocess.run([nssm_path, "set", "sing-box", "AppParameters", 'run -c "C:/profiles/home"'], ...)` +- Good: `subprocess.run([nssm_path, "set", "sing-box", "AppParameters", 'run -c "C:/profiles/home"'], capture_output=True, check=False, text=True, encoding="utf-8", errors="replace")` - Base: profile 路径包含空格时，`AppParameters` 仍作为单个参数传给 NSSM。 - Bad: `subprocess.run(f"nssm.exe set sing-box AppParameters run -c {profile_path}", shell=True)` @@ -85,6 +87,7 @@ nssm.exe remove sing-box confirm - 配置服务测试断言 `Application` 与 `AppParameters` 分别写入。 - NSSM 失败测试断言非零退出码转换为 CLI 错误。 - NSSM 缺失测试断言错误信息说明 `nssm.exe` 不在 `PATH`。 +- 默认 runner 测试断言非法 UTF-8 输出不会触发 `UnicodeDecodeError`，并以替换字符保留在 stdout/stderr 中。 ### 7. Wrong vs Correct @@ -97,7 +100,14 @@ subprocess.run(f"nssm.exe start {SERVICE_NAME}", shell=True) #### Correct ```python -subprocess.run([nssm_path, "start", SERVICE_NAME], capture_output=True, check=False, text=True) +subprocess.run( + [nssm_path, "start", SERVICE_NAME], + capture_output=True, + check=False, + text=True, + encoding="utf-8", + errors="replace", +) ``` ## Ruff 工具链契约 diff --git a/.trellis/tasks/archive/2026-05/05-12-fix-windows-subprocess-output-decoding/check.jsonl b/.trellis/tasks/archive/2026-05/05-12-fix-windows-subprocess-output-decoding/check.jsonl new file mode 100644 index 0000000..9dd3234 --- /dev/null +++ b/.trellis/tasks/archive/2026-05/05-12-fix-windows-subprocess-output-decoding/check.jsonl @@ -0,0 +1 @@ +{"_example": "Fill with {\"file\": \"\", \"reason\": \"\"}. Put spec/research files only — no code paths. Run `python3 .trellis/scripts/get_context.py --mode packages` to list available specs. Delete this line once real entries are added."} diff --git a/.trellis/tasks/archive/2026-05/05-12-fix-windows-subprocess-output-decoding/implement.jsonl b/.trellis/tasks/archive/2026-05/05-12-fix-windows-subprocess-output-decoding/implement.jsonl new file mode 100644 index 0000000..9dd3234 --- /dev/null +++ b/.trellis/tasks/archive/2026-05/05-12-fix-windows-subprocess-output-decoding/implement.jsonl @@ -0,0 +1 @@ +{"_example": "Fill with {\"file\": \"\", \"reason\": \"\"}. Put spec/research files only — no code paths. Run `python3 .trellis/scripts/get_context.py --mode packages` to list available specs. Delete this line once real entries are added."} diff --git a/.trellis/tasks/archive/2026-05/05-12-fix-windows-subprocess-output-decoding/prd.md b/.trellis/tasks/archive/2026-05/05-12-fix-windows-subprocess-output-decoding/prd.md new file mode 100644 index 0000000..96623d3 --- /dev/null +++ b/.trellis/tasks/archive/2026-05/05-12-fix-windows-subprocess-output-decoding/prd.md @@ -0,0 +1,48 @@ +# Fix Windows Subprocess Output Decoding + +## Goal + +Fix `sing start ` and `sing stop` on Windows so NSSM subprocess output does not crash background reader threads when the system locale is GBK and the child process emits bytes that are not valid GBK. + +## Requirements + +- Decode NSSM subprocess stdout and stderr with a deterministic encoding instead of the Windows locale default. +- Keep NSSM failures explicit: non-zero exit codes must still raise `ExternalCommandError` with stderr or stdout content. +- Do not introduce shell command strings, retries, mock success paths, or silent command degradation. +- Cover the default runner behavior with tests. + +## Acceptance Criteria + +- [ ] `default_runner()` calls `subprocess.run()` with explicit `encoding`. +- [ ] `default_runner()` handles undecodable bytes without raising `UnicodeDecodeError` from subprocess reader threads. +- [ ] Existing NSSM command construction and error propagation tests still pass. +- [ ] Ruff, ty, and pytest pass for the changed code. + +## Definition of Done + +- Tests added or updated for the decoding behavior. +- Lint, type check, and tests are green. +- No documentation changes are needed unless command behavior changes. + +## Technical Approach + +Use explicit UTF-8 decoding for captured subprocess output and configure decode errors to replace invalid byte sequences. NSSM output is only used for status checks and human-readable error summaries, so replacement exposes problematic bytes without crashing the CLI. + +## Decision (ADR-lite) + +Context: `subprocess.run(..., text=True)` uses the platform default encoding when no `encoding` is provided. On Windows with a GBK locale, NSSM or service output can include bytes that are not valid GBK, causing `UnicodeDecodeError` in subprocess reader threads. + +Decision: Set `encoding="utf-8"` and `errors="replace"` in the shared `default_runner()`. + +Consequences: Captured output remains text for existing service logic, invalid byte sequences are visible as replacement characters, and command failures continue to surface through the existing error path. + +## Out of Scope + +- Changing NSSM command arguments or service semantics. +- Adding retries or service state recovery. +- Adding a logging framework. + +## Technical Notes + +- Relevant files: `src/sing_cli/service.py`, `tests/test_service.py`. +- Relevant specs: `.trellis/spec/backend/error-handling.md`, `.trellis/spec/backend/quality-guidelines.md`, `.trellis/spec/backend/logging-guidelines.md`. diff --git a/.trellis/tasks/archive/2026-05/05-12-fix-windows-subprocess-output-decoding/task.json b/.trellis/tasks/archive/2026-05/05-12-fix-windows-subprocess-output-decoding/task.json new file mode 100644 index 0000000..fba56b4 --- /dev/null +++ b/.trellis/tasks/archive/2026-05/05-12-fix-windows-subprocess-output-decoding/task.json @@ -0,0 +1,26 @@ +{ + "id": "fix-windows-subprocess-output-decoding", + "name": "fix-windows-subprocess-output-decoding", + "title": "Fix Windows subprocess output decoding", + "description": "", + "status": "completed", + "dev_type": null, + "scope": null, + "package": null, + "priority": "P2", + "creator": "pixelcola", + "assignee": "pixelcola", + "createdAt": "2026-05-12", + "completedAt": "2026-05-12", + "branch": null, + "base_branch": "main", + "worktree_path": null, + "commit": null, + "pr_url": null, + "subtasks": [], + "children": [], + "parent": null, + "relatedFiles": [], + "notes": "", + "meta": {} +} \ No newline at end of file diff --git a/.trellis/workspace/pixelcola/index.md b/.trellis/workspace/pixelcola/index.md index ec85b15..6ac7de0 100644 --- a/.trellis/workspace/pixelcola/index.md +++ b/.trellis/workspace/pixelcola/index.md @@ -8,7 +8,7 @@ - **Active File**: `journal-1.md` -- **Total Sessions**: 3 +- **Total Sessions**: 4 - **Last Active**: 2026-05-12 @@ -19,7 +19,7 @@ | File | Lines | Status | |------|-------|--------| -| `journal-1.md` | ~106 | Active | +| `journal-1.md` | ~139 | Active | --- @@ -29,6 +29,7 @@ | # | Date | Title | Commits | Branch | |---|------|-------|---------|--------| +| 4 | 2026-05-12 | Fix Windows subprocess output decoding | `e90a676` | `fix/windows-subprocess-output-decoding` | | 3 | 2026-05-12 | Use NSSM for Windows service | `72c1b0d` | `fix/use-nssm-service` | | 2 | 2026-05-12 | Windows sing-box CLI | `2f18462` | `feature/windows-sing-box-cli` | | 1 | 2026-05-12 | Bootstrap Guidelines | `ced349d` | `main` | diff --git a/.trellis/workspace/pixelcola/journal-1.md b/.trellis/workspace/pixelcola/journal-1.md index 028dd6b..6265149 100644 --- a/.trellis/workspace/pixelcola/journal-1.md +++ b/.trellis/workspace/pixelcola/journal-1.md @@ -104,3 +104,36 @@ Replaced direct sc.exe service management with NSSM commands, updated service te ### Next Steps - None - task complete + + +## Session 4: Fix Windows subprocess output decoding + +**Date**: 2026-05-12 +**Task**: Fix Windows subprocess output decoding +**Branch**: `fix/windows-subprocess-output-decoding` + +### Summary + +Created PR #4 for the Windows subprocess decoding fix. Updated NSSM subprocess output capture to use explicit UTF-8 replacement handling, added regression coverage, and recorded the backend decoding contract. + +### Main Changes + +(Add details) + +### Git Commits + +| Hash | Message | +|------|---------| +| `e90a676` | (see git log) | + +### Testing + +- [OK] (Add test results) + +### Status + +[OK] **Completed** + +### Next Steps + +- None - task complete diff --git a/src/sing_cli/service.py b/src/sing_cli/service.py index 0eaef9d..dc1691a 100644 --- a/src/sing_cli/service.py +++ b/src/sing_cli/service.py @@ -15,7 +15,7 @@ def default_runner(command: list[str]) -> subprocess.CompletedProcess[str]: - return subprocess.run(command, capture_output=True, check=False, text=True) + return subprocess.run(command, capture_output=True, check=False, text=True, encoding="utf-8", errors="replace") def ensure_windows() -> None: diff --git a/tests/test_service.py b/tests/test_service.py index 966aef9..b2015b8 100644 --- a/tests/test_service.py +++ b/tests/test_service.py @@ -1,4 +1,5 @@ import subprocess +import sys from pathlib import Path import pytest @@ -21,6 +22,15 @@ def enable_windows(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr(service.shutil, "which", lambda executable: f"C:/tools/{executable}") +def test_default_runner_replaces_invalid_utf8_output() -> None: + result = service.default_runner( + [sys.executable, "-c", "import sys; sys.stdout.buffer.write(b'valid' + bytes([0x82]) + b'output')"] + ) + + assert result.returncode == 0 + assert result.stdout == "valid\ufffdoutput" + + def test_configure_service_uses_argument_list(monkeypatch: pytest.MonkeyPatch) -> None: enable_windows(monkeypatch) commands: list[list[str]] = []