Skip to content

feat(cli): thread Config through the cache-sync path - #196

Open
ecalifornica wants to merge 1 commit into
robert/config-cmd-listfrom
robert/config-sync
Open

feat(cli): thread Config through the cache-sync path#196
ecalifornica wants to merge 1 commit into
robert/config-cmd-listfrom
robert/config-sync

Conversation

@ecalifornica

@ecalifornica ecalifornica commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Part of #184. Stacked on #195. path-cli only.

Why

  • The sync commands take &Config. The sync engine takes the config directory, the one value it uses. The missing-directory error surfaces at command entry instead of inside sync.
  • providers::harness_bundle(config) builds the sync providers, so construction stays in providers.

Open question

  • Upgrade config_dir: &Path to a ConfigDir newtype? The document-cache PR later in the stack needs the same directory and would take the same type.

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown

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

Comment thread crates/path-cli/src/sync/engine.rs Outdated
observer: &mut dyn SyncObserver,
) -> Result<Vec<(ArtifactType, SyncOutcome)>> {
let manifest = load_manifest()?;
let manifest = load_manifest(config)?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it make sense to, instead of plumbing the whole config, splitting the parts out that are specific earlier in this call chain? "load_manifest" doesn't really need all of the values in the overall Config, does it?

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.

Done in the updated diff: the engine takes config_dir: &Path, resolved once at the command layer. Open question in the PR body: a ConfigDir newtype instead of the bare &Path.

cmd_cache and cmd_query resolve the config directory once and build
the HarnessBundle through providers::harness_bundle. The manifest
layer takes config_dir; manifest_path is infallible. The transitional
record surfaces load Config and resolve the directory per call.
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