Repository navigation
fix: harden proxy, cleanup, daemon, and import paths from history review - #195
Merged
Merged
Conversation
Proxy handling: - Split bare WinHTTP PAC failover lists (`host:port; host2:port`) on ';' and use the first entry instead of passing an invalid proxy to curl. - Map WinINET protocol keys to curl schemes: socks->socks4, socks4, socks5, socks5h; keep http/https. - Translate '*.domain' bypass entries to curl noproxy suffixes and stop leaking the proxy URL when noproxy allocation fails. - Replace the system-proxy probe spin loop with a blocking Io.Mutex. - scutil --proxy: keep only the nine keys the parser consumes so unrelated fields can no longer evict HTTP/HTTPS/SOCKS settings. Reliability: - `codex-auth app` falls back to the cached codext CLI when the release lookup fails instead of hard-failing offline. - `clean` preserves the managed `codext-cli` cache and default `backup` export directory; stale entries are collected before deletion so iterating-while-deleting cannot skip siblings (same fix for removed account backup pruning). - Rewrites to the user's `config.toml` go through a temp-file rename so a crash cannot leave a truncated file. - The daemon executable resolver skips empty PATH segments and reports not-found instead of falling back to a bare name the OS would resolve against the working directory; a missing executable maps to the silent not_running contract rather than an 'unconfirmed' warning. - The app-running probe distinguishes 'not running' from probe failures so a failed check no longer risks launching a second instance. - Live TUI Enter on a non-selectable row keeps the view open. Validation & safety: - `import --alias` now shares the `alias set` rules (format + conflicts) and validates before writing snapshots or mutating the registry. - Display text from API/auth sources is sanitized (control characters and invalid UTF-8 become spaces) before reaching the terminal; email normalization is shared instead of duplicated. - New app error codes are registered in the preflight handled-error table so failures report friendly messages. - PATH executable checks require the execute bit on POSIX so a data file cannot shadow a real tool. Tests cover PAC failover, socks mapping, bypass translation, scutil overflow, clean whitelist, collect-then-delete, import alias rules, and display sanitization.
…paths - config.toml rewrite resolves symlinks to their target and uses a single atomic rename on POSIX so an interrupted write can no longer leave the settings file missing; Windows keeps the backup-swap dance. - Stored account_name and alias values are cleaned on registry load: names get control characters stripped, and a control-containing alias is dropped instead of silently renamed. - installManagedCodextExecutable prints the package-layout error before returning CodextReleaseInstallFailed so the handled error is not mute. - The stale-entry collectors free the duplicated name when list append fails, closing an error-path leak in clean and account backup pruning. - The Windows app-running probe now also requires a successful exit before trusting the PowerShell output, matching the macOS branch.
…g helpers - A rejected --alias on the single aliased array item now fails the import, so a purge run never saves a registry missing that account. - ExecutableRequired no longer maps blindly to not_running: when the control socket still exists the daemon state stays unconfirmed in both the switch path and the live daemon worker.
Contributor
 [Medium impact] The changes affect several bounded codex-auth workflows for proxy use, account management, and app launching. The PR still needs the Windows settings replacement fixed before merging. Findings
Fix with agent prompt### Issue 1
src/workflows/app.zig:533-539
On Windows, `writeConfigFileWindowsReplace` moves `config.toml` to a backup before putting the new file in place. If the process stops between those renames, `config.toml` stays missing because `errdefer` never runs. Codex can then start without the user's settings. Use a Windows replacement operation that keeps the destination present throughout the swap.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryThis PR hardens proxy selection, cleanup, imports, app setup, and daemon checks.
Reviews (2) · Last reviewed commit: "test: seed a managed stub executable for..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+533
to
+539
| const had_original = blk: { | ||
| std.Io.Dir.cwd().rename(path, std.Io.Dir.cwd(), backup_path, app_runtime.io()) catch |err| switch (err) { | ||
| error.FileNotFound => break :blk false, | ||
| else => return err, | ||
| }; | ||
| break :blk true; | ||
| }; |
Contributor
There was a problem hiding this comment.
On Windows, writeConfigFileWindowsReplace moves config.toml to a backup before putting the new file in place. If the process stops between those renames, config.toml stays missing because errdefer never runs. Codex can then start without the user's settings. Use a Windows replacement operation that keeps the destination present throughout the swap.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/workflows/app.zig
Line: 533-539
Comment:
**Interrupted swaps leave settings missing**
On Windows, `writeConfigFileWindowsReplace` moves `config.toml` to a backup before putting the new file in place. If the process stops between those renames, `config.toml` stays missing because `errdefer` never runs. Codex can then start without the user's settings. Use a Windows replacement operation that keeps the destination present throughout the swap.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
@loongphy/codex-auth-darwin-arm64
@loongphy/codex-auth-darwin-x64
@loongphy/codex-auth-linux-arm64
@loongphy/codex-auth-linux-x64
@loongphy/codex-auth-win32-arm64
@loongphy/codex-auth-win32-x64
@loongphy/codex-auth
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up fixes from the strict v0.3.1..main history review.
Proxy subsystem
host:port; host2:port) on;and use the first entry — previously the whole list reached curl as one invalid--proxyvalue.socks→socks4, plussocks4/socks5/socks5h) and translate*.domainbypass entries to curlnoproxysuffixes.Io.Mutexinstead of a spin loop, and the proxy-URL allocation no longer leaks on the noproxy error path.Reliability
codex-auth appfalls back to the cached codext CLI when the release lookup fails (offline / GitHub rate-limit), instead of hard-failing.cleanpreserves the managedcodext-clicache and the defaultbackupexport directory; stale entries are collected before deletion (also in removed-account backup pruning) so iterating-while-deleting cannot skip siblings.config.tomlrewrites go through temp-file + rename: symlink-aware target resolution plus atomic replace on POSIX (Windows keeps backup-swap), so a crash can't leave a missing or truncated settings file.ExecutableRequiredmaps tonot_runningonly when the control socket is gone — a live endpoint staysunknown.Validation & display safety
import --aliasshares thealias setrules (format + conflicts) and validates before writing snapshots; a rejected alias on the single aliased array item fails the import so--purgenever saves a registry missing that account.account_name/aliasare cleaned on load, and a control-containing stored alias is dropped rather than silently renamed.aliasFormatIssuerejects the same codepoints.Test plan
zig buildcleanzig build test test-helpers— full suite green (incl. new tests: PAC failover, socks mapping, bypass translation, scutil overflow, clean whitelist, collect-then-delete, import alias rules, sanitize, aliasFormatIssue)zig build -Doptimize=ReleaseSafeCODEX_HOMEe2e:import --alias 123→InvalidAlias;cleanpreservesbackup/+codext-cli/