Skip to content

fix(pi-fff): drop stale mode tool names on reload (#855) - #856

Open
gustav-fff wants to merge 1 commit into
mainfrom
triage-bot/issue-855
Open

fix(pi-fff): drop stale mode tool names on reload (#855)#856
gustav-fff wants to merge 1 commit into
mainfrom
triage-bot/issue-855

Conversation

@gustav-fff

@gustav-fff gustav-fff commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Closes #855

Root cause

registerPendingTools was purely additive (packages/pi-fff/src/index.ts:659-665), while pi hands the pre-reload active list straight back to the rebuilt tool registry (dist/core/agent-session.js:2063: activeToolNames: this.getActiveToolNames()). A /reload after a mode switch therefore keeps the previous mode's names active, and both directions are affected on main.

Relevant detail: pi's default active set is ["read", "bash", "edit", "write"] (dist/core/sdk.js:132) — builtin grep/find are registered but inactive. A leftover grep/find in a FFF-named mode is genuinely the extension's stale activation.

Fix

registerPendingTools now takes the names the final mode did not register and drops them from the active set. FFF names (ffgrep/fffind/fff-multi-grep) are pruned unconditionally — no one else registers them. Override names (grep/find/multi_grep) are pruned only when this session actually ran in override mode, determined from the startup-resolved mode plus every persisted /fff-mode entry, so a builtin grep/find the user enabled via defaultTools survives.

Steps to reproduce

cd packages && bun install --frozen-lockfile
git checkout triage-bot/issue-855 -- pi-fff/test/extension.test.ts
cd pi-fff && bun test test/extension.test.ts

On pre-fix main:

(fail) pi-fff mode switch across /reload > drops override tool names when switching back to a FFF-named mode
(fail) pi-fff mode switch across /reload > drops FFF tool names when switching to override
 26 pass
 2 fail

Expected active tools after override -> tools-and-ui + /reload: ["read","bash","edit","write","ffgrep","fffind"].
Actual on main: ["read","bash","edit","write","grep","find","ffgrep","fffind"] — four search tools, duplicated in pairs.

Manual equivalent (TUI):

pi -e packages/pi-fff/src/index.ts --fff-mode=override
/fff-mode tools-and-ui
/reload
/tool          # main: grep, find, ffgrep, fffind active. fixed: ffgrep, fffind

How verified

cd packages/pi-fff && bun test test/     # 85 pass, 0 fail
cd packages && bun run format:check      # all matched files use the correct format
cd packages && bun run lint              # oxlint clean
cd packages/pi-fff && bun run typecheck  # no new errors

Three tests added modelling pi's real cross-reload behaviour (active list and session entries survive, extension instance does not): both switch directions, plus a guard that a user-enabled builtin grep/find is not pruned when override was never used. typecheck still reports the two pre-existing TS7006 in src/index.ts and the unbuilt @ff-labs/fff-node resolution errors — identical on main.

@dmtrKovalenko one judgment call worth your eyes: telling the extension's grep/find from pi's builtins is impossible by name, so the prune leans on mode history as the evidence. If you would rather have hard provenance, the alternative is persisting the activated names in a session entry.

Overlaps #854, which touches the same function — the prune there becomes redundant with this one.

Automated triage via Gustav. Honk-Honk 🪿

Summary by CodeRabbit

  • Bug Fixes
    • Fixed /reload mode switches so tools from a previous mode no longer remain active.
    • Ensured user-enabled built-in tools are preserved when reloading a session.
    • Improved handling of persisted session modes to keep the active tool list accurate.

Pi carries the active tool list across /reload, and registerPendingTools
only added names, so the previous mode's search tools stayed active.

Prune the names the final mode did not register: FFF names unconditionally
(only this extension registers them), override names only when the session
actually ran in override mode, so a user-enabled builtin grep/find is kept.

Closes #855
@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 010fc3c6-2af8-497c-882a-0d36ecf37e88

📥 Commits

Reviewing files that changed from the base of the PR and between d84c0a1 and 84405a0.

📒 Files selected for processing (2)
  • packages/pi-fff/src/index.ts
  • packages/pi-fff/test/extension.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Changes

Reload tool reconciliation

Layer / File(s) Summary
Persisted mode tracking
packages/pi-fff/src/index.ts
sessionModes collects valid persisted fff-mode entries. prepareSession uses the latest mode and detects persisted override mode.
Active tool reconciliation and regression coverage
packages/pi-fff/src/index.ts, packages/pi-fff/test/extension.test.ts
registerPendingTools removes stale FFF and override names before adding registered names. Reload tests verify mode switches and preservation of user-enabled builtin tools.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 84405

Reloading after a mode switch now removes stale search tools while retaining applicable user-enabled builtin tools. The covered behavior is ready to merge.

Suggested reviewers: dmtrkovalenko, ohlulu, xwilludelu

Sequence Diagram(s)

sequenceDiagram
  participant prepareSession
  participant sessionModes
  participant registerPendingTools
  participant ActiveTools
  prepareSession->>sessionModes: Read persisted fff-mode entries
  sessionModes-->>prepareSession: Return modes in order
  prepareSession->>registerPendingTools: Pass stale FFF and override names
  registerPendingTools->>ActiveTools: Remove stale names and append registered names
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main fix: removing stale mode tool names during reload.
Linked Issues check ✅ Passed The changes satisfy issue #855. They remove stale FFF and override tool names during reload, preserve user-enabled builtin tools when override mode was not used, and add regression tests for both swit…
Out of Scope Changes check ✅ Passed All changes support issue #855. The implementation and regression tests are directly related to stale tool cleanup during reload.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch triage-bot/issue-855

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

pi-fff: switching modes leaves stale tool names active after /reload

1 participant