Skip to content

feat(cli): thread Config through the import path - #198

Open
ecalifornica wants to merge 1 commit into
robert/config-queryfrom
robert/config-import
Open

feat(cli): thread Config through the import path#198
ecalifornica wants to merge 1 commit into
robert/config-queryfrom
robert/config-import

Conversation

@ecalifornica

@ecalifornica ecalifornica commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Part of #184 (PR 6 of the plan there). Stacked on #197. path-cli only.

What

  • cmd_p passes &Config to cmd_import::run in the Import arm. p derive reuses cmd_import::run and is not threaded; it loads a transitional Config at its call site.
  • cmd_import constructs every conversation provider via with_resolver(providers::*_resolver(config)). The git, github, and pathbase arms construct no conversation provider and do not change.
  • The pi arm chains with_sessions_dir when --base is given, so --base wins over the injected home.
  • The derive.rs single-session helpers (derive_claude_session and siblings) take &Config and build their managers from the same resolver functions.
  • record_artifact and record_is_current in the sync engine take &Config. The transitional per-call loads inside them are gone.
  • cmd_share is threaded by its own PR. Its two forced call sites (derive_session, and the record_artifact call in share_explicit) load a transitional Config. A load failure at the record_artifact site degrades like a manifest-write failure.
  • HarnessBundle::from_environment() stays: share still uses it, and the import path never constructs a bundle.

Emscripten

  • cmd_import compiles on emscripten. Its claude/gemini/codex/copilot/pi arms construct providers there from the ungated resolvers.
  • The opencode and cursor arms bail inside their #[cfg(target_os = "emscripten")] blocks before any construction; opencode_resolver and cursor_resolver keep their #[cfg(not(target_os = "emscripten"))] gates, and so do the matching derive.rs helpers.
  • emit uses config only in its native manifest-record calls; it carries expect(unused_variables) on emscripten.

Behavior change

  • p import pi / p derive pi / share of a pi session, Windows only: the CLI injects Config::home_dir(), which falls back to $USERPROFILE. The pi resolver's internal lookup reads $HOME only (pi: resolver home lookup ignores $USERPROFILE #192). With $HOME unset, these resolve %USERPROFILE%\.pi\agent\sessions.

Tests

  • The engine tests pass the jail Config to record_artifact; no engine test reads the ambient environment for it.
  • The p import integration tests run the binary under ScopedHome, so $HOME reaches the resolvers through Config end to end (import_copilot_writes_cache, import_records_manifest_so_sync_skips).
  • The cmd_import unit tests build managers from injected resolvers and call the *_with_manager helpers directly; the signature changes do not touch them.

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 13, 2026

Copy link
Copy Markdown

🔍 Preview deployed: https://e98bb229.toolpath.pages.dev

Comment on lines +18 to +21
// Transitional: `p derive` does not take `&Config` yet; load one
// for the import path.
let config = crate::config::Config::load()?;
crate::cmd_import::run(args, pretty, &config)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

cmd_p already holds &Config, so cmd_derive::run could take it today. Leaving the transitional load in: it carries the same marker as the other unthreaded sites and gets swept with them.

@ecalifornica
ecalifornica marked this pull request as ready for review August 14, 2026 20:14
@ecalifornica
ecalifornica requested a review from akesling August 14, 2026 20:14
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