fix: IPC mode port discovery, empty result handling, shell quoting, and debug support#6
Merged
Timmy6942025 merged 2 commits intomasterfrom Mar 27, 2026
Conversation
…nd debug support
- Fix IPC subprocess connection to fzf's listen port by prepending
FPF_FZF_LISTEN_HOST=localhost:$FZF_PORT in buildDynamicQueryNotifyIPCCommandGo()
- Fall back to checking $FZF_PORT in runIPCReload() if FPF_FZF_LISTEN_HOST is empty
- Remove hardcoded localhost:9812 fallback (incorrect with --listen=0)
- Return early with error to stderr when port cannot be determined
- Fix empty result handling: emit fallback file instead of clearing fzf list
when buildDisplayRows() returns zero results
- Improve shell quoting: use single quotes for {q} placeholder to safely
handle shell metacharacters across bash, dash, and zsh
- Add visual manager scope indicator: show 'Search (fast)> ' when flatpak/npm
are excluded from live search
- Add FPF_DEBUG_RELOAD=1 support for debugging live search issues
- Add tests: IPC port discovery test, debug reload unit test
- Add Linux troubleshooting section to README.md
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds Linux live-search troubleshooting docs, debug logging for dynamic reload, IPC-based reload transport with port discovery from FZF_PORT, adjustments to fzf reload command quoting and prompt behavior, and corresponding test updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant CLI as fpf CLI
participant FZF as fzf (selector)
participant Manager as Managers (detectors)
participant IPC as IPC notify
User->>CLI: type query (live search)
CLI->>Manager: compute manager subset (dynamic reload)
alt subset != all
CLI->>FZF: start with "Search (fast)>" prompt
else
CLI->>FZF: start with "Search>" prompt
end
CLI->>Manager: buildDisplayRows(query, managers)
Manager-->>CLI: rows
CLI->>FZF: send rows (stdin)
opt IPC transport enabled
CLI->>IPC: send query notify (FPF_FZF_LISTEN_HOST/FZF_PORT)
IPC->>FZF: result:change-prompt / reload trigger
end
FZF->>CLI: user selection / reload requests
CLI->>Manager: on full reload -> use all managers
Manager-->>CLI: updated rows
CLI->>FZF: update prompt to original via result:change-prompt
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/fpf/cli_runtime.go (1)
715-728:⚠️ Potential issue | 🔴 CriticalRemove shell quotes around the
{q}placeholder.Wrapping
{q}in quotes ('{q}') causes queries with apostrophes to break. When fzf substitutes{q}with a query likeo'clock, it produces-- 'o'clock'where the shell quote from the placeholder surrounds an unescaped apostrophe in the query, breaking the command. fzf's documented reload examples pass{q}unquoted.Suggested fix
func buildDynamicReloadCommandGo(managerOverride, fallbackFile, managerListCSV string) string { bypass := dynamicReloadBypassValueGo() parts := []string{ "FPF_SKIP_INSTALLED_MARKERS=1", "FPF_BYPASS_QUERY_CACHE=" + bypass, "FPF_SKIP_QUERY_CACHE_WRITE=1", "FPF_IPC_MANAGER_OVERRIDE=" + shellQuoteIfNeeded(managerOverride), "FPF_IPC_MANAGER_LIST=" + shellQuoteIfNeeded(managerListCSV), "FPF_IPC_FALLBACK_FILE=" + shellQuoteIfNeeded(fallbackFile), shellQuote(os.Args[0]), "--dynamic-reload", "--", - "'{q}'", + "{q}", } return strings.Join(parts, " ") } func buildDynamicQueryNotifyIPCCommandGo(managerOverride, fallbackFile, managerListCSV string) string { parts := []string{ "FPF_FZF_LISTEN_HOST=localhost:$FZF_PORT", "FPF_IPC_MANAGER_OVERRIDE=" + shellQuoteIfNeeded(managerOverride), "FPF_IPC_MANAGER_LIST=" + shellQuoteIfNeeded(managerListCSV), "FPF_IPC_FALLBACK_FILE=" + shellQuoteIfNeeded(fallbackFile), shellQuote(os.Args[0]), "--ipc-query-notify", "--", - "'{q}'", + "{q}", } return strings.Join(parts, " ") }Also applies to: 732-742
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/fpf/cli_runtime.go` around lines 715 - 728, In buildDynamicReloadCommandGo, remove the surrounding shell single quotes around the fzf query placeholder so queries with apostrophes don't break; locate the parts entry that currently contains "'{q}'" and change it to "{q}" (and make the same change for the other identical occurrence later in the function), ensuring the placeholder is passed unquoted so fzf can substitute queries like o'clock correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/fpf/cli_runtime.go`:
- Around line 214-216: The "(fast)" prompt is shown whenever the manager lists
differ (currently computed by comparing joined strings), which also flips for
reordered or custom subsets; change the logic that sets excludeSlowManagers
(used when calling runFuzzySelectorGo) to only be true when reloadCmd and
reloadFullCmd are set AND dynamicReloadManagers(managers) is a strict subset of
managers (every element of dynamic exists in managers and len(dynamic) <
len(managers)) instead of any inequality; also ensure the prompt-change bindings
(the result:change-prompt / change handlers that run after reload) do not
unconditionally overwrite the indicator—make those handlers consult the same
excludeSlowManagers flag (or accept it as a parameter) so the prompt remains
correct after the first reload.
In `@cmd/fpf/ipc_actions.go`:
- Around line 48-52: The fallback that sets fzfHost from os.Getenv("FZF_PORT")
should accept either a bare port or a precomposed host:port; change the logic
where fzfPort is read (variable fzfPort in cmd/fpf/ipc_actions.go, used to set
fzfHost) to check whether fzfPort contains ":" and if so assign fzfHost =
fzfPort directly, otherwise assign fzfHost = "localhost:"+fzfPort so you don't
produce addresses like "localhost:127.0.0.1:9999".
---
Outside diff comments:
In `@cmd/fpf/cli_runtime.go`:
- Around line 715-728: In buildDynamicReloadCommandGo, remove the surrounding
shell single quotes around the fzf query placeholder so queries with apostrophes
don't break; locate the parts entry that currently contains "'{q}'" and change
it to "{q}" (and make the same change for the other identical occurrence later
in the function), ensuring the placeholder is passed unquoted so fzf can
substitute queries like o'clock correctly.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f19e2a50-e60d-4472-b65b-025da5beb13a
📒 Files selected for processing (8)
README.mdcmd/fpf/cli_runtime.gocmd/fpf/cli_runtime_test.gocmd/fpf/dynamic_reload.gocmd/fpf/dynamic_reload_test.gocmd/fpf/ipc_actions.gocmd/fpf/perf_trace_test.gotests/smoke.sh
📜 Review details
🧰 Additional context used
🪛 GitHub Actions: test
tests/smoke.sh
[error] 1-1: fpf: missing packaged Go binary for linux/x86_64. Expected one of: /home/runner/work/fpf-cli/fpf-cli/bin/fpf-go-linux-amd64 or /home/runner/work/fpf-cli/fpf-cli/bin/fpf-go-linux-arm64. Run 'bash scripts/build-go-binaries.sh' to build local binaries.
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 3 file(s) based on 2 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 3 file(s) based on 2 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
This was referenced Mar 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes several issues with the live search and IPC mode functionality:
IPC Mode Port Discovery Fix
FPF_FZF_LISTEN_HOST=localhost:\$FZF_PORTto the IPC command so the subprocess correctly connects to fzf's listen port\$FZF_PORTenvironment variable ifFPF_FZF_LISTEN_HOSTis not setlocalhost:9812fallback which was incorrect with--listen=0Empty Result Handling Fix
buildDisplayRows()returns zero results, the code now callsemitFile(fallbackFile)instead ofwriteBuildDisplayRowsTSV(rows)to preserve the current fzf list rather than clearing itShell Quoting Improvement
"{q}"to'{q}'(single quotes) in both reload commands for safer shell handling across bash, dash, and zshVisual Manager Scope Indicator
Search (fast)>prompt when flatpak/npm are excluded from live search to signal to users that these managers are excluded and Ctrl+R will search all managersDebug Support
FPF_DEBUG_RELOAD=1environment variable support that logs to stderr: query received, managers being searched, result count, and any errors encounteredTests Added
run_dynamic_reload_ipc_port_discovery_test()in smoke.sh to verifyFPF_FZF_LISTEN_HOSTappears in IPC command outputTestDebugReloadEnabledunit test for debug flag parsingDocumentation
FPF_DYNAMIC_RELOAD_MANAGERS=allfor full coverageFPF_DEBUG_RELOAD=1