Add Pi activity tracking plugin - #3594
Conversation
Signed-off-by: Aditya Datta <crazyme07071996@gmail.com>
worktrunk-bot
left a comment
There was a problem hiding this comment.
Reviewing as a draft — flagging anything that looks worth a quick fix. Mark ready for a full review.
Nice work — the pi.rs handler and the dev/pi-plugin.ts hook closely mirror the existing opencode plugin, which is the right pattern to follow. A few things I'd tighten before this goes green:
1. The parent wt config plugins help + snapshot are out of sync. Adding the Pi variant to ConfigPluginsCommand changes the wt config plugins --help command list, but tests/snapshots/…__help_config_plugins.snap still enumerates only claude/codex/opencode — so test_help_config_plugins (and test_docs_are_in_sync) will fail in CI. The snapshots regenerate via the pre-merge hook (cargo run -- hook pre-merge --yes, or cargo insta test --accept -- --test integration "test_help").
That regen won't fix the authored prose, though: the Plugins command's after_long_help in src/cli/config.rs (the ## Supported tools list and ## Examples, near the Install and manage Worktrunk plugins for AI coding tools. block) still omits Pi. That one's a hand edit — add:
- **pi** — Pi / oh-my-pi plugin (activity tracking)
to the Supported tools list, and $ wt config plugins pi install to the Examples block.
2. PI_CODING_AGENT_DIR precedence when a profile is active. In pi_agent_dir(), PI_CODING_AGENT_DIR is honored only when active_profile().is_none() — an active $OMP_PROFILE/$PI_PROFILE silently overrides an explicit $PI_CODING_AGENT_DIR. I can't verify Pi's own resolution order from CI, but "explicit dir override loses to profile" reads backwards from the usual "most-explicit wins" expectation. Worth confirming against Pi's actual behavior (and a one-line comment in the code recording why, since it's non-obvious).
3. Hook doesn't tolerate wt failing. The opencode hook guards every call with || true so a wt failure (not in a managed repo, wt not on PATH) never propagates into the host. The Pi hook does a bare await pi.exec("wt", …). If pi.exec rejects on a non-zero exit, an agent_start in a non-worktrunk directory would surface an error into the Pi session. Does pi.exec reject on non-zero exit, or is it fire-and-forget? If it rejects, wrap the body in a try/catch (or the Pi equivalent of || true).
4. Test coverage is lighter than opencode's. The opencode suite covers the already-installed, prompt-declined, and uninstall-not-installed branches; the Pi tests cover only the install/uninstall happy paths (and only OMP_PROFILE, not PI_PROFILE or PI_CONFIG_DIR). Those extra branches are exactly what codecov/patch tends to flag — worth mirroring the opencode tests before marking ready.
Minor: handle_pi_uninstall uses std::fs::remove_file — identical to the accepted opencode uninstall (removes only the tool's own hook at a fixed path), so low-risk, but flagging it since it's on worktrunk's data-safety radar. @max-sixty for eventual eyes on the deletion path.
|
Thanks for the draft review. The parent help/snapshots, Pi directory precedence, |
Summary
wt config plugins pi install|uninstallValidation
cargo fmt --check— passedcargo test --test integration test_pi_ -- --nocapture— 3 passedINSTA_UPDATEcargo clippy --bin wt -- -D warnings— passedCloses #3571