Skip to content

fix: preserve Windows paths for Codex resume#214

Merged
vakovalskii merged 1 commit into
vakovalskii:mainfrom
IamXesus:fix/windows-codex-resume-paths
May 15, 2026
Merged

fix: preserve Windows paths for Codex resume#214
vakovalskii merged 1 commit into
vakovalskii:mainfrom
IamXesus:fix/windows-codex-resume-paths

Conversation

@IamXesus
Copy link
Copy Markdown
Contributor

Что исправлено

Исправляет Resume для Codex-сессий на Windows, когда проект лежит по Windows-пути вроде C:\projects.

До фикса при нажатии Resume могла открываться консоль в C:\Windows\System32 или сервер отклонял запуск с ошибкой
invalid or unsafe project path.

Причина

Было два связанных слоя проблемы:

  • frontend передавал project path через inline onclick, где обратные слэши в C:\... интерпретировались как JS
    escape-последовательности;
  • Windows launcher строил команду через вложенный cmd /k "cd \"...\" && ...", что ломало quoting и не гарантировало
    корректный working directory.

Изменения

  • Добавлен JS string escaping для project path в inline handlers.
  • Native Windows запуск переведён на Start-Process -WorkingDirectory.
  • Windows Terminal теперь получает cwd через --startingDirectory.
  • Добавлены regression tests для Windows path escaping и Windows launcher args.

Проверка

  • node --test test\frontend-escaping.test.js test\terminals-windows-launch.test.js

Также запускал полный node --test: все новые проверки проходят, остаётся существующий Windows-only fail из-за
EPERM на fs.symlinkSync в test/git-root-resolve.test.js.

Copy link
Copy Markdown
Owner

@vakovalskii vakovalskii left a comment

Choose a reason for hiding this comment

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

LGTM ✅ Welcome to the project @IamXesus, nice first contribution!

Verified:

  • node -c clean on terminals.js, app.js, detail.js
  • 5/5 new tests pass (frontend-escaping.test.js, terminals-windows-launch.test.js)

Spot-checks:

escJsString:

  • Backslash escaped first (essential — otherwise subsequent \' escapes would be double-escaped)
  • \r, \n, U+2028, U+2029 covered (the last two are easy to miss but break JS string literals)
  • Final escHtml wrap handles the HTML-attribute layer — double-encoding round-trips correctly:
    • Input C:\\Users\\fooescJsStringC:\\\\Users\\\\foo → browser HTML-decode → C:\\Users\\foo → JS string parse → C:\\Users\\foo (single backslashes), correct

Windows launcher:

  • Refactor extracts buildAgentCommand + per-launcher arg builders as pure functions — testable, no exec side-effects in unit tests 👍
  • Start-Process -WorkingDirectory is the right pattern; eliminates the nested cmd /k "cd \"...\" && ..." quoting fragility
  • quotePowerShellSingle correctly uses '' (doubled single quote) as PowerShell's literal-string escape
  • Switching the three Windows paths from exec (shell) to execFileSafe (no shell) is a security improvement on top of the bug fix
  • Windows Terminal: --startingDirectory is the documented cwd flag, matches MS docs
  • -NoProfile on PowerShell — good practice (no user-profile side effects)

Thanks for the carefully-scoped fix + regression tests!

@vakovalskii vakovalskii merged commit 7c148be into vakovalskii:main May 15, 2026
@IamXesus IamXesus deleted the fix/windows-codex-resume-paths branch May 15, 2026 07:59
@vakovalskii vakovalskii mentioned this pull request May 25, 2026
3 tasks
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.

2 participants