Skip to content

Add Pi activity tracking plugin - #3594

Open
adity982 wants to merge 8 commits into
max-sixty:mainfrom
adity982:agent/add-pi-activity-plugin
Open

Add Pi activity tracking plugin#3594
adity982 wants to merge 8 commits into
max-sixty:mainfrom
adity982:agent/add-pi-activity-plugin

Conversation

@adity982

Copy link
Copy Markdown

Summary

  • add wt config plugins pi install|uninstall
  • install a profile-aware Pi / oh-my-pi hook with atomic writes
  • map Pi lifecycle events to Worktrunk working, waiting, and cleared activity markers
  • honor Pi's agent-directory, config-directory, and profile environment variables
  • cover profile paths, explicit directory overrides, uninstall behavior, and CLI output snapshots

Validation

  • cargo fmt --check — passed
  • cargo test --test integration test_pi_ -- --nocapture — 3 passed
  • the same snapshot tests passed again without INSTA_UPDATE
  • cargo clippy --bin wt -- -D warnings — passed

Closes #3571

Signed-off-by: Aditya Datta <crazyme07071996@gmail.com>

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

@adity982

Copy link
Copy Markdown
Author

Thanks for the draft review. The parent help/snapshots, Pi directory precedence, pi.exec failure semantics, and the missing install/uninstall/profile branches all need to be checked before this is ready. I will keep the PR in draft while those items are addressed.

max-sixty pushed a commit that referenced this pull request Aug 18, 2026
…thout a plugin (#3848)

## Problem

#3847 asks for a documented "generic agent" integration: worktrunk ships
plugins for Claude Code, Codex, OpenCode, and Gemini, so users of any
other agent CLI have no documented way to get the 🤖/💬 activity markers
in `wt list`. The mechanism is already agent-agnostic — the plugins just
call `wt config state marker` on their host's session events — but the
docs only present manual markers as a personal-workflow convenience, so
users reverse-engineer the integration from that section. #3571 (pi /
oh-my-pi) is the same gap from a different host.

## Solution

A new **Agent CLIs without a plugin** subsection under Activity tracking
in
[`docs/content/claude-code.md`](https://github.com/max-sixty/worktrunk/blob/main/docs/content/claude-code.md),
stating the three-call contract (set 🤖 on session start, set 💬 on turn
end, clear on session end) plus the three things that actually bite:

- the command resolves the branch from its working directory, so the
hook must run inside the worktree (`--branch` where the host pins cwd
elsewhere);
- `marker set` exits non-zero outside a repository, and hosts differ on
what a non-zero hook does — guard it;
- pair every set with a clear, and expect a stale marker if the process
is killed first.

Docs-only. The skill and plugin-skill mirrors are regenerated by the
sync test.

## Testing

`cargo test --test integration test_docs_are_in_sync` passes (it
regenerated both mirrors, committed here).

Each claim in the section was verified against a scratch repo with a
linked worktree rather than taken from the existing prose:

<details><summary>Verification</summary>

```
$ wt config state marker set "🤖"          # from /tmp/mrepo.feature-x
✓ Set marker for feature-x to 🤖
$ git config --get worktrunk.state.feature-x.marker
{"marker":"🤖","set_at":1787044121}
```

- Works from a subdirectory of the worktree (branch still resolves to
`feature-x`).
- Outside a repository: `✗ git rev-parse --git-common-dir failed (exit
128)`, exit code 1 — the basis for the "guard it" bullet.
- `marker clear` with no marker set exits 0 (`○ No marker set for
main`), so a session-end hook is safe to run unconditionally.
- `wt list` renders the marker in the Status column as documented.

</details>

## Scope

Deliberately host-agnostic. The reporter's second ask — a native `wt
config plugins copilot` target — is a maintainer call and isn't
attempted here: GitHub Copilot CLI does expose the needed events
(`sessionStart` / `agentStop` / `sessionEnd`, user-level hooks under
`~/.copilot/hooks/`, per the [hooks
reference](https://docs.github.com/en/copilot/reference/hooks-reference)),
but nothing in CI can drive a Copilot session to verify a generated hook
file end to end. A concrete Copilot config is posted on the issue for
the reporter to confirm; if it works, adding it here as a worked example
is a natural follow-up. #3594 (native `pi` target) is the adjacent
in-flight work and doesn't overlap with this.

---
Refs #3847 — automated triage

---------

Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>
@adity982

Copy link
Copy Markdown
Author

Addressed the draft-review items on current main:

  • added Pi to the parent wt config plugins help, examples, and snapshot;
  • made explicit PI_CODING_AGENT_DIR take precedence over active profiles;
  • isolated pi.exec failures so activity tracking cannot interrupt the host session;
  • added coverage for PI_PROFILE + PI_CONFIG_DIR, override/profile precedence, repeated install, and uninstall-when-missing.

Focused validation:

  • cargo test --test integration pi_ — 7 passed
  • cargo clippy --tests -- -D warnings — passed
  • cargo fmt --all — clean

The Unix help snapshot test module is intentionally disabled on Windows, so I updated the reviewed snapshot directly; the refreshed Linux CI run will verify its rendered form. The PR remains draft while that CI completes.

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewing as a draft — flagging anything that looks worth a quick fix. Mark ready for a full review.

My previous item 2 sent you the wrong way — sorry. I asked you to confirm the PI_CODING_AGENT_DIR-vs-profile precedence against Pi's actual behavior, and the new commit resolved it by dropping the guard. Pi's own docs say the opposite: PI_CODING_AGENT_DIR is a "Full agent-directory override for the default profile only; named profiles ignore it" (environment-variables.md), restated in config-usage.md as "Named profiles ignore PI_CODING_AGENT_DIR." So the original .filter(|_| active_profile().is_none()) was correct and removing it is a regression: with OMP_PROFILE=research and PI_CODING_AGENT_DIR both set, Pi loads hooks from ~/.omp/profiles/research/agent/hooks/pre/, but the installer now writes to $PI_CODING_AGENT_DIR/hooks/pre/worktrunk.ts and reports success — the hook never loads and no markers ever appear. Inline suggestions restore the guard (with the comment recording why), flip test_pi_agent_dir_override_takes_precedence_over_profile to pin the upstream behavior, and correct the --help text.

Everything else in the resolution order checks out against those docs: active_profile()'s "OMP_PROFILE wins even when explicitly empty, and default/empty/whitespace means the default profile" matches config-usage.md exactly, PI_CONFIG_DIR as a dirname under $HOME matches, and <agent-dir>/hooks/pre/* is the right user-level hook root.

codecov/patch is red at 95.4% on src/commands/config/pi.rs — the four missed lines are the two prompt-declined return Ok(()) branches (install and uninstall). That's the one branch from my earlier item 4 that isn't covered yet; test_opencode_install_prompt_declined is the pattern to mirror — no --yes, piped stdin declines the prompt.

The user-facing plugin docs don't mention Pi. docs/src/content/docs/claude-code.md ("Agent Integration") is the primary hand-edited page listing every plugin — Pi needs a column in the capability table, an ### Pi block under Installation naming ~/.omp/agent/hooks/pre/worktrunk.ts and the env vars, and a mention in the frontmatter description and intro paragraph. skills/worktrunk/reference/claude-code.md derives from it, so don't edit that copy. This isn't caught by CI (check-docs passes — the generated regions are unaffected), but CLAUDE.md treats behavior changes as requiring doc updates.

Two smaller things I'd fold in rather than leave: wt config show renders a section per agent tool (render_opencode_status and friends, keyed off a which::which probe), and Pi has none — an installed-but-outdated Pi hook is invisible there, and the "Plugin not installed. To install, run …" hint is how the feature gets discovered. And confirm_or_yes is now byte-identical in opencode.rs and pi.rs; worth lifting into the parent config module while there are only two copies.

Test hermeticity note

set_temp_home_env pins OPENCODE_CONFIG_DIR and CLAUDE_CONFIG_DIR so those tests can't be steered by the developer's own environment, and isolate_subprocess_env scrubs GIT_*/WORKTRUNK_*. Nothing scrubs OMP_PROFILE, PI_PROFILE, PI_CONFIG_DIR, or PI_CODING_AGENT_DIR, so test_pi_install_is_idempotent, test_pi_uninstall_removes_hook, and test_pi_install_creates_profile_aware_hook all assert against .omp/agent/... paths that an ambient OMP_PROFILE in the runner's shell would move. test_pi_install_honors_pi_profile_and_config_dir already env_removes one of them, which is the tell. Adding those four to the env_remove list in set_temp_home_env (src/testing/mod.rs, beside the CLAUDE_CONFIG_DIR line) fixes all of them at once — the tests that want a value set it afterwards, and the later cmd.env wins.

Comment thread src/commands/config/pi.rs
Comment thread tests/integration_tests/config_show.rs Outdated
Comment thread tests/integration_tests/config_show.rs Outdated
Comment thread src/cli/config.rs Outdated
Signed-off-by: Aditya Datta <crazyme07071996@gmail.com>
@adity982

adity982 commented Aug 25, 2026

Copy link
Copy Markdown
Author

Coverage follow-up:

  • Codecov first identified four uncovered lines in the install/uninstall declined-prompt paths; focused tests in 10a5e46e0 reduced that to two instrumentation-only misses on the multiline )? { expressions.
  • 1d7e7ba4d now uses the same let confirmed = ...; if !confirmed? control flow as the existing fully-covered OpenCode installer, without changing behavior.
  • cargo test --test integration pi_ passes all 9 Pi cases and cargo clippy --tests -- -D warnings is clean on the latest head.

The hosted patch-coverage check is rerunning on 1d7e7ba4d.

Signed-off-by: Aditya Datta <crazyme07071996@gmail.com>
Signed-off-by: Aditya Datta <crazyme07071996@gmail.com>
@worktrunk-bot

Copy link
Copy Markdown
Collaborator

Coverage is green on 1d7e7ba4d: codecov/patch reports 100.00% of diff hit (target 98.13%), and codecov/project is 98.14% (+0.00%). The let confirmed = …; if !confirmed? reshape did close the two instrumentation-only misses, so that thread is done. test (linux), test (macos), and test (windows) all passed on that head too — only the advisory affected tests (macos|windows) jobs were still running when I looked, and they don't gate.

The profile-guard restoration in b9fdcd33f is right, and thanks for taking the correction on my bad steer — pi_agent_dir() now matches Pi's documented resolution in both directions, and the renamed regression test pins it.

Four items from my last review are still open, listed here so the finish line is visible rather than as new asks:

  1. User-facing docs. docs/src/content/docs/claude-code.md is the hand-edited page listing every plugin and still has no Pi — capability-table column, an ### Pi block naming ~/.omp/agent/hooks/pre/worktrunk.ts and the env vars, plus the frontmatter description and intro. check-docs passes because the generated regions are unaffected, so CI won't catch this one. Don't edit skills/worktrunk/reference/claude-code.md; it derives from that page.
  2. wt config show has no Pi section. show.rs:73 renders a block per agent tool behind a which::which probe; without a render_pi_status an outdated-or-missing Pi hook is invisible, and the "Plugin not installed. To install, run …" hint is how the feature gets discovered.
  3. confirm_or_yes is byte-identical in opencode.rs:63 and pi.rs:56 — worth lifting into the parent config module while there are only two copies.
  4. Test hermeticity. Still nothing scrubs OMP_PROFILE / PI_PROFILE / PI_CONFIG_DIR / PI_CODING_AGENT_DIR, so the Pi tests asserting .omp/agent/... paths can be steered by an ambient profile in the runner's shell.
Where the hermeticity fix goes

set_temp_home_env pins OPENCODE_CONFIG_DIR and CLAUDE_CONFIG_DIR at src/testing/mod.rs:1205-1209 for exactly this reason. Adding the four Pi vars to an env_remove beside that CLAUDE_CONFIG_DIR line covers test_pi_install_is_idempotent, test_pi_uninstall_removes_hook, and test_pi_install_creates_profile_aware_hook in one edit — the tests that want a value set it afterwards via cmd.env, which wins over the removal.

@adity982
adity982 marked this pull request as ready for review August 26, 2026 07:55

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Full (non-draft) pass on 1d7e7ba4d. The code itself reads well — pi_agent_dir() matches Pi's documented resolution in both directions, the hook is failure-isolated via try/catch, the marker vocabulary (agent_start/agent_end/session_shutdown → 🤖/💬/clear) lines up with the Claude and OpenCode hooks, and codecov/patch is green. Two things keep this from an approval.

Holding for @max-sixty — deletion surface. handle_pi_uninstall calls std::fs::remove_file (src/commands/config/pi.rs, the Failed to remove plugin @ {target_display} line). This repo's review policy lists std::fs::remove_file as a hold trigger — .claude/skills/running-tend/references/review-pr.md, "Data-Loss Surface: Hold for Human Review" — so a new plugin surface that deletes a file isn't mine to sign off, even though it is the same shape as the already-accepted handle_opencode_uninstall (fixed path, the tool's own hook, behind a prompt). @max-sixty for the call.

Four gaps from the draft rounds are still open on this head. I'd normally not restate, but you've marked it ready, so here's the finish line. Item 2 is wider than I scoped it before; the rest are unchanged.

  1. User-facing docs. docs/src/content/docs/claude-code.md is the hand-edited "Agent Integration" page enumerating every plugin, and Pi appears nowhere in it — no column in the capability table, no ### Pi block under Installation, and the frontmatter description still reads "Claude Code, Codex, OpenCode, and Gemini CLI". CLAUDE.md treats behavior changes as requiring doc updates, and nothing in CI catches this one: the generated regions are unaffected, so check-docs and test_docs_are_in_sync both pass. skills/worktrunk/reference/claude-code.md derives from that page — don't edit the copy.

  2. The duplication is the whole module, not just confirm_or_yes. Comparing src/commands/config/pi.rs against src/commands/config/opencode.rs: confirm_or_yes is byte-identical, and handle_pi_install/handle_pi_uninstall differ from their OpenCode counterparts only in the tool name inside the prompt string and the path resolver they call — the already-installed short-circuit, the Update/Install selection, the preview closure, create_dir_all + write_atomically, the success + hint lines, and the not-installed early return are all reproduced verbatim. That's ~80 lines with two owners. The drift has already started: wt config plugins pi uninstall prints Plugin removed @ <path> where the OpenCode command prints Plugin removed from <path> for the identical operation (visible in the two *_uninstall_* snapshots). A shared helper parameterized by (display name, plugin source, resolved target) collapses both into their *_dir() functions and stops the next divergence.

  3. wt config show has no Pi section. show.rs renders a block per agent tool — render_claude_code_status, render_codex_status, render_opencode_status, render_gemini_status, each behind a which::which probe — and there's no render_pi_status. opencode.rs exports is_plugin_installed() and plugin_file_exists() for exactly that three-way status; pi.rs exports neither. Concretely: a user who installs the Pi hook today and upgrades wt next month has a stale worktrunk.ts that wt config show says nothing about, and the Plugin outdated. To update, run … / Plugin not installed. To install, run … hints — which are how the feature gets discovered at all — never fire for Pi.

  4. Test hermeticity. Nothing scrubs OMP_PROFILE, PI_PROFILE, PI_CONFIG_DIR, or PI_CODING_AGENT_DIR. On a machine where the developer exports OMP_PROFILE=work, test_pi_install_is_idempotent, test_pi_install_prompt_declined, test_pi_uninstall_removes_hook, test_pi_uninstall_prompt_declined, and test_pi_install_honors_agent_dir_override all resolve to ~/.omp/profiles/work/agent/... and fail their .omp/agent/... assertions. test_pi_install_honors_pi_profile_and_config_dir already env_removes OMP_PROFILE, which is the tell. CI runners don't set these, so this is a local-developer failure, not a red build.

Where the hermeticity fix goes

set_temp_home_env in src/testing/mod.rs pins OPENCODE_CONFIG_DIR and CLAUDE_CONFIG_DIR (the cmd.env("CLAUDE_CONFIG_DIR", home.join(".claude")) line) for exactly this reason. Adding the four Pi vars to an env_remove beside it covers all five tests in one edit — the tests that want a value set it afterwards via cmd.env, which wins over the removal.

Happy to push a commit for 1 and 4 if you'd like — both are mechanical and I've already worked out the content. Say the word and I'll do it; 2 and 3 are yours since they involve design choices about where the shared helper lives and what the Pi availability probe should be (Pi's binary name isn't obvious to me from CI).

Signed-off-by: Aditya Datta <crazyme07071996@gmail.com>
@adity982

Copy link
Copy Markdown
Author

Addressed the four non-draft follow-ups on the current head: documented Pi in the agent-integration capability table and install section; extracted the shared OpenCode/Pi file-plugin install/uninstall flow; added Pi current/outdated/missing status to wt config show using the pi executable probe; and scrubbed OMP_PROFILE, PI_PROFILE, PI_CONFIG_DIR, and PI_CODING_AGENT_DIR in the shared test-home setup. Validation: Pi integration 10/10, OpenCode regression 12/12, Pi config-show states 3/3, cargo clippy --tests -- -D warnings, Astro check with 0 diagnostics, and docs tests 9/9. The remove_file policy hold remains intentionally unresolved for @max-sixty's human review; I did not try to override that repository-policy gate.

@worktrunk-bot

Copy link
Copy Markdown
Collaborator

All four follow-ups landed as described — I checked each on 083bcba17: the capability column plus ### Pi block and frontmatter in docs/src/content/docs/claude-code.md, install_file_plugin/uninstall_file_plugin extracted into src/commands/config/mod.rs (which also closes the Plugin removed @ vs Plugin removed from drift I flagged), render_pi_status in show.rs, and OMP_PROFILE/PI_PROFILE/PI_CONFIG_DIR/PI_CODING_AGENT_DIR scrubbed in set_temp_home_env. The remove_file hold stays with @max-sixty — agreed that isn't yours to clear.

083bcba17 is red on three checks though, and all three are deterministic consequences of that commit rather than flakes. None of them would show up in the focused runs you listed.

1. check_stderr_macros_come_from_styling — fails in test (linux), and it's also what aborted code-coverage (nextest stops on first failure, so no coverage report was uploaded and codecov/patch has no result on this head yet). The guard reports all seven eprintln! sites in the new shared helper. The imports are right, but they're function-local: mod.rs:45 and mod.rs:99. The scan only counts use statements at column 0 — output_system_guard.rs:385 spells out why, and predicts exactly this: "a future function-local one fails this scan and gets hoisted." Both functions import the same superset, so one top-level use worktrunk::styling::{eprintln, hint_message, info_message, success_message}; beside the pub use block replaces both inner imports.

2. test_docs_are_in_sync — also test (linux). Editing the hand-authored claude-code.md leaves three generated files stale: skills/worktrunk/reference/claude-code.md, plugins/worktrunk/skills/worktrunk/reference/claude-code.md, and docs/public/llms.txt. The test rewrites them in place, so cargo test --test integration test_docs_are_in_sync locally and committing the result is the whole fix.

3. check-docs — the built-site suite, not the Rust docs tests. built-site.test.mjs:478 pins the capability table by its exact header list, asserting it isn't rendered as responsive records; the new Pi column makes the lookup miss and the assertion reads is missing table Capability | Claude Code | Codex | OpenCode | Gemini CLI. Add 'Pi' between 'OpenCode' and 'Gemini CLI' there. This file is hand-maintained, so it won't regenerate — verify with cd docs && npm run build && npm run test:site.

Worth running cargo run -- hook pre-merge --yes before the next push: it covers the full integration suite (both Rust failures) in one go, which cargo test --test integration pi_ scoped past.

Where each failure came from
  • test (linux) run 330867050874731 tests run: 4729 passed, 2 failed; the two are output_system_guard::check_stderr_macros_come_from_styling and readme_sync::test_docs_are_in_sync.
  • code-coverage same run groupCancelling due to test failure after the same guard test, at 2704/4731. Same root cause as Add CI workflows and fix cross-platform test failures #1; it should go green once that's fixed, and only then does codecov/patch get a number.
  • check-docs✖ short wide tables become labeled records without capturing dense tables, AssertionError: /claude-code/ is missing table …, tests 13 / pass 12 / fail 1.
  • test (macos) and test (windows) were still running when I looked; they run the same suite, so expect the same two.

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

All four follow-ups from the last pass landed — the shared install_file_plugin/uninstall_file_plugin helper, the Pi section in wt config show, the hand-edited claude-code.md page, and the four env vars scrubbed in set_temp_home_env. The uninstall wording drift is gone too, now that both tools print through the same helper.

Three things are red on 083bcba17, and I reproduced each one locally on the merged tree.

1. code-coverage is failing the test run, not the coverage bar. check_stderr_macros_come_from_styling rejects the seven new eprintln! sites in src/commands/config/mod.rs, because the guard only counts use worktrunk::styling::… statements at column 0 and the helper's imports are function-local. The guard's own doc comment on styling_imports calls this shape out — "a future function-local one fails this scan and gets hoisted". Inline suggestions hoist all four use lines to the module top; they're one edit, so apply all three together. Verified locally: cargo clippy --all-targets -- -D warnings clean, cargo fmt --check clean, and the guard test passes.

2. check-docs is failing on a hardcoded copy of the capability table's header. docs/tests/built-site.test.mjs asserts the /claude-code/ table by exact header list, and the new Pi column moves it — /claude-code/ is missing table Capability | Claude Code | Codex | OpenCode | Gemini CLI. Find the findTable('/claude-code/', …) call in the short wide tables become labeled records without capturing dense tables test and add the column:

    findTable('/claude-code/', ['Capability', 'Claude Code', 'Codex', 'OpenCode', 'Pi', 'Gemini CLI']).attributes,

With that one line, npm run test:site goes 13/13 here. It's outside the diff so I can't suggest it inline — happy to push it if you'd rather not round-trip.

3. test_docs_are_in_sync will fail the test (linux|macos|windows) legs. claude-code.md is primary, so its three derived copies need regenerating and none are in the diff:

Files out of sync (updated):
  [skill files] skills/worktrunk/reference/claude-code.md
  [plugin skills mirror] plugins/worktrunk/skills/worktrunk/reference/claude-code.md
  [llms.txt] docs/public/llms.txt

cargo test --test integration test_docs_are_in_sync writes them, then fails; a second run passes. Those legs were still in flight when I looked, so this is a local reproduction rather than a CI report.

And one that CI can't catch: the availability probe names a binary that doesn't exist. is_pi_available() calls which::which("pi"), but oh-my-pi ships omp, not pipackages/coding-agent/package.json declares "bin": { "omp": "src/cli.ts" }, and https://omp.sh/install writes the release asset to ${INSTALL_DIR}/omp. So on a real install the PI heading never renders, and the Plugin not installed. To install, run … / Plugin outdated… hints — which are the whole reason for adding the section — never fire. test_config_show_pi_plugin_status stays green because it sets WORKTRUNK_TEST_PI_INSTALLED=1 and never exercises the which path. Upstream badlogic/pi-mono does ship a pi binary, but everything else here targets the fork: ~/.omp, $OMP_PROFILE, and the @oh-my-pi/pi-coding-agent type import. Inline suggestion switches the probe.

The rest of the resolution checks out against upstream. ~/.omp/agent/hooks/pre/ is the right user hook root (config-usage.md: user root ~/.omp/agent/..., hooks at hooks/pre/*), agent_start / agent_end / session_shutdown are all real events in docs/hooks.md, and pi.exec(command, args, { cwd }) matches HookAPI.exec — it returns { code } rather than throwing on non-zero, so the try/catch is covering spawn failures, which is the case that matters.

How I verified the four findings
  • code-coverage: job logFAIL … check_stderr_macros_come_from_styling, listing src/commands/config/mod.rs:{54,68,82,86,105,113,121}, then 2027/4731 tests were not run due to test failure. Hoisting the imports locally turns it green.
  • check-docs: ran the docs-build action's steps by hand (npm ci, npm run check, npm test, npm run build, npm run test:site) — only test:site fails, at tests/built-site.test.mjs:442.
  • doc sync: cargo test --test integration test_docs_are_in_sync on the merged tree, output quoted above.
  • binary name: gh api repos/can1357/oh-my-pi/contents/packages/coding-agent/package.json and curl -fsSL https://omp.sh/install. packages/{metaharness,stats,mnemopi} declare metaharness, omp-stats, and mnemopi; no package in the repo declares a pi bin.

Still holding rather than approving, for the std::fs::remove_file reason in my last pass — that's unchanged, just moved into uninstall_file_plugin. @max-sixty for that call. Patch coverage is unverified on this head, since the code-coverage job died before uploading.

Comment thread src/commands/config/mod.rs
Comment thread src/commands/config/mod.rs
Comment thread src/commands/config/mod.rs
if let Ok(val) = std::env::var("WORKTRUNK_TEST_PI_INSTALLED") {
return val == "1";
}
which::which("pi").is_ok()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

oh-my-pi installs its CLI as omp"bin": { "omp": "src/cli.ts" } in packages/coding-agent/package.json, and the install script writes ${INSTALL_DIR}/omp. With pi here the whole PI block is unreachable on a real install.

Suggested change
which::which("pi").is_ok()
which::which("omp").is_ok()

While you're here: is_opencode_available and render_opencode_status both carry a doc line (/// Check if OpenCode CLI is available, /// Render OPENCODE section (plugin status). plus the Caller must check … note), and the two Pi functions have none. Worth matching, and the same goes for install_file_plugin / uninstall_file_plugin next to the documented run_plugin_cli.

Signed-off-by: Aditya Datta <crazyme07071996@gmail.com>
@adity982

Copy link
Copy Markdown
Author

Fixed the three deterministic failures in 3fd304497: hoisted the styling imports for the stderr-macro guard, regenerated the Pi documentation mirrors plus llms.txt, and updated the built-site capability-table expectation. Validation: check_stderr_macros_come_from_styling passed 1/1; Astro production build passed; the focused wide-table built-site regression passed 1/1. The full built-site suite passed 12/13 locally, with only the fresh-checkout missing /assets/social/social-card.png asset check failing (unrelated to this diff). The docs generator produced the expected three files; its only Windows-only stop was the .well-known/agent-skills/worktrunk symlink check.

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The three doc/import findings from the last pass all landed, and I re-ran each locally on the merged tree: check_stderr_macros_come_from_styling passes, test_docs_are_in_sync passes with a clean tree afterwards, and the built-site.test.mjs header now matches claude-code.md's actual | Capability | Claude Code | Codex | OpenCode | Pi | Gemini CLI |. check-docs, code-coverage, test (linux) and lint are green; macOS/Windows still in flight.

codecov/patch is red, and two of the four missed lines are a real regression the refactor introduced. Patch is 139/143 = 97.2% against an auto target of 98.13% (the base's project coverage), so it fails by 4 lines. Only two of them are yours to fix, and fixing them alone clears the gate (141/143 = 98.6%).

The two that matter are src/commands/config/mod.rs:67 and :110 — the || eprintln!("{}", preview_msg), argument in each helper. On main this was a named binding on its own line (let preview = || eprintln!("{}", preview_msg); in opencode.rs), and codecov scored that line a hit: the statement that creates the closure runs on every install, even though the body only runs when the user types ? at the prompt. Inlining the closure as a call argument collapses the line to the closure body's region, which no test reaches — so the refactor turned two covered lines into two misses. Restoring the binding restores the hits without adding a test. Inline suggestions do that; I applied both locally and cargo fmt --check and cargo clippy --bin wt -- -D warnings stay clean.

The other two, src/commands/config/show.rs:385 and :393, aren't fixable and I don't think you should chase them. They're the )?; lines closing the writeln! calls in render_pi_status's "Plugin outdated" and "Plugin not installed" arms — the error-propagation region of a writeln! into a String, which cannot fail. All three arms are exercised (test_config_show_pi_plugin_status's three rstest cases hit the function 3 times), and the byte-identical lines in render_opencode_status are codecov misses on main today. They only count here because the function is new.

Still holding rather than approving, unchanged from the last pass: uninstall_file_plugin calls std::fs::remove_file, which this repo's review reference lists as a hold-for-human trigger. @max-sixty for that call. The is_pi_available() thread on show.rs is also still open — no new argument from me, it just hasn't been picked up.

How I got the coverage numbers

Codecov's API was intermittently rate-limited, so I cross-checked two ways and they agree exactly.

  • cobertura.xml from the code-coverage artifact on this head (run 33241342622), intersected with git diff -U0 bb421ec...3fd3044: misses at mod.rs:{67,110} and show.rs:{385,393}, everything else in pi.rs, main.rs, opencode.rs, testing/mod.rs at 100%.
  • GET /api/v2/github/max-sixty/repos/worktrunk/compare/?pullid=3594totals.patch = {lines: 143, hits: 139, misses: 4, coverage: 97.2}, totals.base.coverage = 98.13; per-file mod.rs 96.55%, show.rs 92.0%, pi.rs 100%.
  • The main-side comparison: file_report/src/commands/config/opencode.rs?sha=bb421ec reports the file at 100% with lines 87–88 (the preview_msg / let preview = … pair) as hits, while file_report/src/commands/config/show.rs?sha=bb421ec reports lines 346 and 354 — the )?; closers in render_opencode_status — as misses.

Comment on lines +63 to +68
let preview_msg = info_message(cformat!("Would write to <bold>{target_display}</>"));
let confirmed = yes
|| prompt_yes_no_preview(
&cformat!("{action} {name} plugin @ <bold>{target_display}</>?"),
|| eprintln!("{}", preview_msg),
)? == PromptResponse::Accepted;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let preview_msg = info_message(cformat!("Would write to <bold>{target_display}</>"));
let confirmed = yes
|| prompt_yes_no_preview(
&cformat!("{action} {name} plugin @ <bold>{target_display}</>?"),
|| eprintln!("{}", preview_msg),
)? == PromptResponse::Accepted;
let preview_msg = info_message(cformat!("Would write to <bold>{target_display}</>"));
let preview = || eprintln!("{}", preview_msg);
let confirmed = yes
|| prompt_yes_no_preview(
&cformat!("{action} {name} plugin @ <bold>{target_display}</>?"),
preview,
)? == PromptResponse::Accepted;

Binding the closure first puts a statement on the line that runs on every install, which is what opencode.rs did on main and why codecov counted it. As an inline argument the line maps only to the closure body, which nothing reaches.

Comment on lines +106 to +111
let preview_msg = info_message(cformat!("Would remove <bold>{target_display}</>"));
let confirmed = yes
|| prompt_yes_no_preview(
&cformat!("Remove {name} plugin @ <bold>{target_display}</>?"),
|| eprintln!("{}", preview_msg),
)? == PromptResponse::Accepted;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let preview_msg = info_message(cformat!("Would remove <bold>{target_display}</>"));
let confirmed = yes
|| prompt_yes_no_preview(
&cformat!("Remove {name} plugin @ <bold>{target_display}</>?"),
|| eprintln!("{}", preview_msg),
)? == PromptResponse::Accepted;
let preview_msg = info_message(cformat!("Would remove <bold>{target_display}</>"));
let preview = || eprintln!("{}", preview_msg);
let confirmed = yes
|| prompt_yes_no_preview(
&cformat!("Remove {name} plugin @ <bold>{target_display}</>?"),
preview,
)? == PromptResponse::Accepted;

Same for the uninstall helper — this is the second of the two patch misses.

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.

[FEATURE] Support Pi / Oh My Pi coding agent

2 participants