refactor(cli): one ArtifactType layer and per-session import plumbing by benbaarber · Pull Request #133 · empathic/toolpath

benbaarber · 2026-07-17T16:55:38Z

First of a four-PR stack splitting up #131 into reviewable pieces, per review. This one is the harness/type groundwork; the sync engine, query auto-sync, and --parent-dir scoping follow in the stack.

One ArtifactType layer. ArtifactType (in artifact.rs) becomes the single enum naming artifact sources — it absorbs the former HarnessArg, and --harness filters and import cache-id prefixes all route through it. The Harness enum stays as the harness-only layer that share/resume --harness take (you can't resume into a git repo), mapping into the general enum via Harness::artifact_type(). SessionRow generalizes to ArtifactRow with an optional message_count.

Cross-command machinery gets its own modules (per review — thanks @akesling): artifact.rs holds ArtifactType; harness.rs holds Harness, HarnessBundle, and the provider not-found probes; cache.rs holds the document store (write_cached, cache_ref, list_cached, make_id, …); derive.rs holds DerivedDoc, the per-session derive_*_session helpers, and the Pathbase fetch. cmd_*.rs files keep only their command's argument surface and flow — nothing imports machinery out of another command's file anymore.

Per-session import plumbing. Every import flow — explicit --session, picker multi-select, --all, and the most-recent fallbacks — now loops per-session derive helpers (derive_*_session_with, one per provider, each usable with an injected provider manager). --all warns-and-skips unreadable sessions instead of aborting the batch.

Breaking (pre-1.0): p import pi --all emits one Path document per session, consistent with every other provider (previously a single combined Graph). p import gemini loses --include-thinking — thinking is always ingested; the cache, uploads, and resume all carry the full derivation.

Claude path.base fix. Claude derives leave DeriveConfig.project_path unset so path.base comes from the session's own recorded cwd rather than the lossy project-dir slug, falling back to the caller's --project when no cwd was recorded.

Library groundwork (consumed by the sync PR next in the stack): toolpath-gemini 0.6.1 adds PathResolver::list_session_entries and bounds peek_session_id to a 4 KiB prefix scan with full-parse fallback; toolpath-claude 0.12.1 makes ClaudeConvo::session_chain public (rides the cached chain index, no extra IO after a listing). path-cli 0.15.0 → 0.16.0.

Tests: cargo clippy -D warnings clean; 433 tests pass at this point in the stack.

^{Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.}

Stack (review bottom-up): #133 types/imports → #134 cache sync → #135 query auto-sync → #136 parent-dir. Supersedes #131.

Unify harness/artifact typing: ArtifactType is the single enum naming artifact sources (absorbing HarnessArg), Harness stays as the harness-only layer share/resume take, SessionRow generalizes to ArtifactRow. Every import flow now loops per-session derive helpers (derive_*_session_with, injectable managers) — breaking: p import pi --all emits one Path per session; p import gemini loses --include-thinking (thinking is always ingested). Claude derives source path.base from the session's recorded cwd instead of the lossy dir slug. Library groundwork: toolpath-gemini 0.6.1 (list_session_entries, bounded peek_session_id), toolpath-claude 0.12.1 (session_chain public).

github-actions · 2026-07-17T17:04:45Z

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

akesling

Generally looks good. Let's clean up the dependencies/ownership for Harness and ArtifactType in the CLI, though.

akesling · 2026-07-17T19:10:33Z

+#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, clap::ValueEnum)]
 #[value(rename_all = "lower")]
-pub enum HarnessArg {
+pub enum Harness {


Why does cmd_share own Harness and this impl ArtifactType when they're used by other commands, etc.? Why do we have this impl here anyway when ArtifactType is owned by sync?

Would it make sense to push this to some common types/helpers module and then have everything depend on that?

Extracted to src/artifact.rs and src/harness.rs.

ArtifactType lived in sync.rs and Harness/HarnessBundle in cmd_share.rs while being consumed across import, share, resume, and cache. Give the shared layers their own modules: artifact.rs holds ArtifactType, harness.rs holds Harness, ArtifactType::harness(), HarnessBundle, and the provider is_not_found_* probes. Command modules keep only their own logic.

…oc links The old sync module was pub, so its ArtifactType doc links resolved as public items. Keep that visibility for the split-out modules, and refer to the pub(crate) HarnessBundle without a link.

…ommands The document-cache store (cache_path/write_cached/cache_ref/list/ remove/make_id) and single-session derivation (DerivedDoc, the derive_*_session helpers, the Pathbase fetch) were owned by cmd_cache.rs and cmd_import.rs while being called from share, resume, export, and query. Give them their own modules; the command files keep only their arg surfaces and flows.

The cursor and opencode session helpers are cfg'd off on emscripten (SQLite), and cmd_import's remaining make_id/ArtifactType/doc_inner_id uses all sit in gated code — the unconditional imports broke or warned on the site's wasm build, which quality_gates.sh doesn't cover.

benbaarber · 2026-07-20T18:07:52Z

additionally fixed that antipattern across the cli crate, extracted logic into derive.rs and cache.rs instead of exporting logic from cmd files.

akesling

A couple things to improve, but LGTM

akesling · 2026-07-23T17:23:13Z

+            ArtifactType::Cursor => "cursor  ",
+            ArtifactType::Pi => "pi      ",
+            ArtifactType::Copilot => "copilot ",
+            ArtifactType::Git => "git     ",


What are all of these spaces?

this was column padding for fzf. replaced with a width constant and a padding function.

akesling · 2026-07-23T17:44:04Z

+/// The trailing UUID of a codex rollout filename stem
+/// (`rollout-<timestamp>-<uuid>`), or the whole stem when it doesn't end
+/// in one. Codex's `read_session` resolves either form.
+fn codex_artifact_id(stem: &str) -> &str {


Why is this even inspecting the ID? Shouldn't functionality related to this be owned by the codex crate? Maybe we need extract ID creation out to some helper indirection for all the harnesses?

Codex --all now works like the other providers

akesling · 2026-07-23T17:53:18Z

@@ -45,9 +45,9 @@ use clap::Args;
 use std::path::PathBuf;

 /// Re-exported so external callers (integration tests, future consumers)


Let's remove this comment.

akesling · 2026-07-23T17:54:24Z

            // re-fetches; `--no-cache` skips both the probe AND the post-
            // fetch write (still useful for ephemeral environments).
-            let cache_id = crate::cmd_import::pathbase_cache_id_of(u, args.url.as_deref())?;
+            let cache_id = crate::derive::pathbase_cache_id_of(u, args.url.as_deref())?;


I think cache ID management probably be owned by the cache module.

akesling · 2026-07-23T17:59:32Z

+    }
+}
+
+pub(crate) fn is_not_found_claude(err: &toolpath_claude::ConvoError) -> bool {


We can leave this be here, but definitely calling out this manual reification smell as something we should maybe revisit in the future.

…h, pathbase key in cache - Replace the hand-padded symbol() literal table with padded_name() formatting over NAME_COLUMN_WIDTH; the picker row matcher compares the first token against name() instead of prefix-matching padding. - Delete codex_artifact_id: the codex --all import reads each enumerated rollout file directly via derive_codex_session_file instead of round-tripping filename -> id -> re-glob. - Drop the stale Harness re-export comment in cmd_resume along with the re-export itself; tests import path_cli::harness::Harness. - cache.rs owns the pathbase cache-key scheme (pathbase_cache_id); pathbase_cache_id_of is gone — resume parses the ref and computes the key in two visible steps.

Codex --all now lists session metadata and derives by id like every other provider, instead of enumerating rollout files and deriving by path (drops derive_codex_session_file; one derive helper per provider again). opencode and cursor --all now warn-and-skip unreadable sessions like the other five providers instead of aborting the batch.

…ll imports toolpath-codex 0.6.1: list_session_ids returns rollout filename stems from one directory walk with no file reads, and find_rollout_file resolves a full stem straight to its sessions/YYYY/MM/DD path before falling back to the tree walk. p import codex --all now lists ids that way, so the symmetric list-then-derive shape costs one parse per file and one walk total — cheaper than the old filename-parsing version.

akesling approved these changes Jul 17, 2026

View reviewed changes

benbaarber added 6 commits July 20, 2026 11:11

style(cli): cargo fmt

e5f79cc

fix(cli): make artifact and harness modules pub for rustdoc's intra-d…

c8d8d7d

…oc links The old sync module was pub, so its ArtifactType doc links resolved as public items. Keep that visibility for the split-out modules, and refer to the pub(crate) HarnessBundle without a link.

docs: record the module extraction in the changelog

894c296

benbaarber assigned akesling Jul 20, 2026

benbaarber requested a review from akesling July 20, 2026 18:07

akesling approved these changes Jul 23, 2026

View reviewed changes

benbaarber added 3 commits July 23, 2026 14:31

benbaarber merged commit 192d726 into main Jul 23, 2026
2 checks passed

This was referenced Jul 24, 2026

fix(claude): drop empty streaming-seed assistant lines on projection #141

Open

feat(resume): path resume --remote <ssh-url> — resume a session on a remote host #145

Open

		@@ -45,9 +45,9 @@ use clap::Args;
		use std::path::PathBuf;

		/// Re-exported so external callers (integration tests, future consumers)

Uh oh!

Conversation

benbaarber commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akesling left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

benbaarber commented Jul 20, 2026

Uh oh!

akesling left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

benbaarber commented Jul 17, 2026 •

edited

Loading

github-actions Bot commented Jul 17, 2026 •

edited

Loading