Skip to content

refactor(codex): PathResolver::new takes a required home argument - #207

Open
ecalifornica wants to merge 2 commits into
robert/config-geminifrom
robert/config-codex
Open

refactor(codex): PathResolver::new takes a required home argument#207
ecalifornica wants to merge 2 commits into
robert/config-geminifrom
robert/config-codex

Conversation

@ecalifornica

@ecalifornica ecalifornica commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Part of #185: the crate reads no environment variable, Config owns every read, and the caller supplies what the crate used to resolve itself.

PathResolver::new(home) takes the home directory as a required argument. The crate keeps the layout knowledge (<home>/.codex); the caller owns "what is home". With the home always present, the pure path accessors drop their Result and the Default impls go with them.

Judgment calls:

  • read_session(path) stays lenient. Context: a rollout file is a Codex session log (~/.codex/sessions/**/rollout-*.jsonl), and a writer can leave a truncated last line. The short spelling of the API keeps the crash-tolerant read; strict is an explicit opt-in via with_strict(bool) or read_session_with(path, strict).
  • Strict is presence-based: $CODEX_ROLLOUT_STRICT set, whatever its value. This matches the deleted std::env::var(...).is_ok() check exactly. Config stores the raw Option<String>; providers::codex_strict owns the presence-is-the-signal interpretation. The variable keeps its behavior for CLI users.
  • A missing home degrades differently by surface: providers::codex_resolver returns Option<PathResolver>, the harness bundle silently omits Codex, and the 5 commands that target Codex error through require_codex_resolver. This is a behavior decision, not part of the mechanical refactor.
  • The second commit deletes CodexDirectoryNotFound separately: it turned out to be constructed nowhere, so its removal is independent of the home change.

Both commits are breaking for toolpath-codex and ride one 0.7.0 bump. The CHANGELOG lists the affected API.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown

🔍 Preview deployed: https://3586681d.toolpath.pages.dev

@ecalifornica
ecalifornica marked this pull request as ready for review August 17, 2026 17:06
@ecalifornica
ecalifornica requested a review from akesling August 17, 2026 17:06
@ecalifornica ecalifornica assigned akesling and unassigned akesling Aug 17, 2026
toolpath-codex reads no environment variable. The caller supplies the
home directory; the crate keeps the layout below it (`<home>/.codex`).

Library (breaking, 0.7.0):

- `PathResolver::new(home)` takes the home directory as a required
  argument. `CodexConvo::new(home)` and `ConvoIO::new(home)` take the
  same argument.
- Delete the `Default` impls on the three types, `with_home`, the
  private `dirs::home_dir` helper, and the `NoHomeDirectory` error
  variant.
- Keep `with_codex_dir` as the full override.
- The home directory is always present, so `home_dir()`, `codex_dir()`,
  `sessions_root()`, `history_file()`, `log_file()`, and
  `ConvoIO::codex_dir_path()` return a path, not a `Result`. Callers
  inside the crate drop the dead error arms.
- Strict rollout parsing is a `bool` parameter.
  `RolloutReader::read_session_with(path, strict)` takes it directly.
  `ConvoIO::with_strict` and `CodexConvo::with_strict` store it and pass
  it down. `RolloutReader::read_session(path)` stays lenient.

path-cli:

- `providers::codex_resolver` returns `Option<PathResolver>`. `None`
  means `Config` carries no home directory, so Codex is out of reach.
- `harness_bundle` omits the Codex manager in that case.
- `providers::require_codex_resolver` reports "cannot determine the home
  directory" for the 5 commands that target Codex.
- `Config` reads `$CODEX_ROLLOUT_STRICT`. `providers::codex_strict`
  turns its presence into the flag every `CodexConvo` receives, so the
  variable keeps its behavior for CLI users.

Version bump per the release checklist: crate manifest, workspace
dependency, site/_data/crates.json, CHANGELOG.md.
…nt (#185)

The variant is declared and matched in path-cli's is_not_found_codex
but constructed nowhere. Deleting a public enum variant is breaking;
it rides the toolpath-codex 0.7.0 bump on this branch.
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