Skip to content

fix(windows): preserve image validation stderr - #115

Closed
nightt5879 wants to merge 1 commit into
Fei-Away:mainfrom
nightt5879:agent/fix-windows-image-metadata-native
Closed

fix(windows): preserve image validation stderr#115
nightt5879 wants to merge 1 commit into
Fei-Away:mainfrom
nightt5879:agent/fix-windows-image-metadata-native

Conversation

@nightt5879

Copy link
Copy Markdown
Contributor

Summary / 摘要

  • Route Windows image-metadata validation through the existing Invoke-DreamSkinNative compatibility wrapper instead of invoking Node with a direct 2>&1 redirection.
  • Discard helper stderr while preserving stdout and the real exit code, so harmless Node warnings cannot terminate PowerShell 5.1 or corrupt the JSON metadata result.
  • Preserve the existing friendly size-safety error for invalid images.
  • Add a real PowerShell 5.1 regression that injects Node stderr on both successful and failing metadata checks.

Root cause / 根因

Windows PowerShell 5.1 promotes redirected native stderr to ErrorRecord objects. With $ErrorActionPreference = 'Stop', the direct image-helper invocation could terminate on its first stderr line before reading $LASTEXITCODE, bypassing the repository's native-command compatibility layer and its stable validation message.

Type / 类型

  • Bug fix / 缺陷修复
  • Feature / 新功能
  • Docs / 文档
  • Theme / CSS / visual / 主题或视觉
  • Scripts / install / restore / 脚本或安装恢复
  • Chore / 杂项

Platform / 平台

  • macOS
  • Windows
  • Both / 双平台
  • Docs / repo only / 仅文档或仓库元数据

Self-check / 自测

Check what you actually ran. Skip items that do not apply and say so under Notes.
请勾选实际跑过的项;不适用的在 Notes 说明。

Docs-only / 仅文档

  • Links and wording reviewed / 已检查链接与表述

macOS (when code under macos/ changes)

  • macos/tests/run-tests.sh passed / 已通过
  • Doctor (optional): macos/scripts/doctor-macos.sh
  • Live verify (if inject/CSS/start path): verify-dream-skin-macos.sh or Desktop Verify
  • Restore / re-apply smoke (if install/restore/start changed) / 若改了安装恢复启动则做过恢复再应用

Windows (when code under windows/ changes)

  • Relevant install / start / verify / restore scripts exercised / 已按改动跑过对应脚本
  • Environment noted below (OS build, Codex source) / 下方注明环境

User-facing / 用户可见变更

  • Updated macos/CHANGELOG.md (and macos/VERSION if release-worthy) / 已更新 changelog(发版时再 bump VERSION)
  • N/A — no user-facing change / 无用户可见变更

Security / 安全

  • Does not modify official Codex install / asar / signatures / 未修改官方安装与签名
  • Does not silently write API Base URL or keys / 未静默写入 API Base URL 或 Key
  • CDP remains loopback-oriented (127.0.0.1) where applicable / CDP 仍仅本机回环(如适用)

Notes / 补充

  • Passed twice: powershell.exe -NoProfile -ExecutionPolicy Bypass -File .\windows\tests\run-tests.ps1.
  • The regression uses a Node data: preload to emit stderr under $ErrorActionPreference = 'Stop'; a valid image still parses, while an invalid image returns the friendly Image metadata is invalid or exceeds... error rather than NativeCommandError.
  • The warning lines in the suite output are expected fault-injection coverage for runtime and atomic-cleanup failure paths.
  • Passed: PowerShell parser checks for the changed scripts and git diff --check.
  • Updated windows/CHANGELOG.md; the template's changelog checkbox is macOS-specific.
  • Environment: Windows 11 build 26200; Windows PowerShell 5.1.26100.8875; Node.js 22.22.2.
  • No live Codex restart or renderer injection was needed because the change is isolated to pre-import image validation.

Fixes #112

@nightt5879
nightt5879 marked this pull request as ready for review July 17, 2026 09:25
@Fei-Away Fei-Away added the status: considering Maintainer likes the idea, evaluating for a future merge label Jul 23, 2026
@Fei-Away

Fei-Away commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Closing — #112 (what this PR was meant to fix) was already resolved through a different change, so this PR has nothing left to land. Thanks for the contribution!

关闭:这个 PR 对应的 #112 已经通过其他改动解决了,这个 PR 不需要再合并了。感谢你的贡献!

@Fei-Away Fei-Away closed this Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: considering Maintainer likes the idea, evaluating for a future merge

Projects

None yet

2 participants