fix: prevent pkill -f self-kill in Makefile targets#5
Merged
DavidsonGomes merged 1 commit intoEvolutionAPI:developfrom Apr 13, 2026
Merged
Conversation
On Linux (procps-ng) with dash as /bin/sh, `pkill -f "pattern"` inside a make recipe matches the recipe shell's own argv (since make runs each line as `sh -c '<line>'`, putting the pattern literally in the shell's cmdline). pkill then SIGTERMs its own parent shell, causing `Terminated` and aborting the recipe before it does anything else. Apply the classic `[p]attern` bracket trick so the regex still matches real target processes (`dashboard/...`, `app.py`) but the literal `[d]ashboard`/`[a]pp.py` string in the shell's own argv does not contain the substring being searched — pkill no longer matches itself. Affects 5 targets: dashboard-app, terminal-stop, stop, uninstall, restore. Reproduces reliably on WSL2 Ubuntu + dash + procps-ng 4.0.4. macOS users likely unaffected because BSD pkill / bash-as-sh handle `sh -c` argv differently, which is why this wasn't caught earlier.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts Makefile pkill invocations to avoid killing their own recipe shell by using the classic bracketed-pattern trick in process-matching regexes, ensuring service stop/cleanup targets work reliably on Linux/dash without self-termination. Sequence diagram for Makefile pkill behavior in dashboard-app targetsequenceDiagram
actor Developer
participant Make
participant Shell
participant Pkill
participant TerminalServer
Developer->>Make: make dashboard-app
Make->>Shell: sh -c cd dashboard/frontend && ... && pkill -f [d]ashboard/terminal-server/bin/server.js
Shell->>Pkill: pkill -f [d]ashboard/terminal-server/bin/server.js
alt Matching terminal-server
Pkill-->>Shell: return success (matched TerminalServer)
Pkill-->>TerminalServer: SIGTERM
else No matching terminal-server
Pkill-->>Shell: return failure (no matches)
end
Note over Shell,Pkill: [d]ashboard pattern ensures Shell argv does not match itself
Shell->>TerminalServer: node dashboard/terminal-server/bin/server.js --dev &
Shell-->>Make: start backend via app.py
Make-->>Developer: dashboard-app running
Flow diagram for Makefile service stop and cleanup targets using bracketed pkill patternsflowchart TD
subgraph MakeTargets
A[dashboard-app] -->|cleanup existing| B["pkill -f [d]ashboard/terminal-server/bin/server.js"]
C[terminal-stop] --> B
D[stop] --> B
D --> E["pkill -f [d]ashboard/backend.*app.py"]
D --> F["pkill -f [a]pp.py"]
G[uninstall] --> B
G --> E
G --> F
H[restore] --> B
H --> F
end
B --> I[Terminate terminal-server processes]
E --> J[Terminate dashboard backend app.py processes]
F --> K[Terminate generic app.py processes]
classDef target fill:#e0f7ff,stroke:#0077aa,stroke-width:1px;
classDef action fill:#fef3c7,stroke:#92400e,stroke-width:1px;
class A,C,D,G,H target;
class B,E,F,I,J,K action;
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="Makefile" line_range="133" />
<code_context>
- pkill -f "app.py" 2>/dev/null || true; \
+ pkill -f "[d]ashboard/terminal-server/bin/server.js" 2>/dev/null || true; \
+ pkill -f "[d]ashboard/backend.*app.py" 2>/dev/null || true; \
+ pkill -f "[a]pp.py" 2>/dev/null || true; \
echo "Removing nginx config..."; \
rm -f /etc/nginx/sites-enabled/evonexus 2>/dev/null || true; \
</code_context>
<issue_to_address>
**issue (bug_risk):** Consider whether the broad `[a]pp.py` pattern might hit unrelated `app.py` processes on the host
`pkill -f "[a]pp.py"` will match any process whose command line contains `app.py`, not just this backend. On shared/dev hosts this could kill unrelated apps. Prefer a more specific pattern (e.g., including the project path, venv name, or a unique CLI argument).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| pkill -f "app.py" 2>/dev/null || true; \ | ||
| pkill -f "[d]ashboard/terminal-server/bin/server.js" 2>/dev/null || true; \ | ||
| pkill -f "[d]ashboard/backend.*app.py" 2>/dev/null || true; \ | ||
| pkill -f "[a]pp.py" 2>/dev/null || true; \ |
There was a problem hiding this comment.
issue (bug_risk): Consider whether the broad [a]pp.py pattern might hit unrelated app.py processes on the host
pkill -f "[a]pp.py" will match any process whose command line contains app.py, not just this backend. On shared/dev hosts this could kill unrelated apps. Prefer a more specific pattern (e.g., including the project path, venv name, or a unique CLI argument).
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
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.
On Linux (procps-ng) with dash as /bin/sh,
pkill -f "pattern"inside a make recipe matches the recipe shell's own argv (since make runs each line assh -c '<line>', putting the pattern literally in the shell's cmdline). pkill then SIGTERMs its own parent shell, causingTerminatedand aborting the recipe before it does anything else.Apply the classic
[p]atternbracket trick so the regex still matches real target processes (dashboard/...,app.py) but the literal[d]ashboard/[a]pp.pystring in the shell's own argv does not contain the substring being searched — pkill no longer matches itself.Affects 5 targets: dashboard-app, terminal-stop, stop, uninstall, restore.
Reproduces reliably on WSL2 Ubuntu + dash + procps-ng 4.0.4. macOS users likely unaffected because BSD pkill / bash-as-sh handle
sh -cargv differently, which is why this wasn't caught earlier.Summary by Sourcery
Bug Fixes: