Skip to content

Fix pkill patterns in CLAUDE.md to prevent killing Copilot Coding Agent#18653

Merged
harupy merged 9 commits intomasterfrom
copilot/fix-pkill-command-issue
Nov 4, 2025
Merged

Fix pkill patterns in CLAUDE.md to prevent killing Copilot Coding Agent#18653
harupy merged 9 commits intomasterfrom
copilot/fix-pkill-command-issue

Conversation

Copy link
Contributor

Copilot AI commented Nov 4, 2025 •
edited
Loading

Fix pkill commands in CLAUDE.md and run-dev-server.sh to prevent killing Copilot Coding Agent

Problem:

  • CLAUDE.md instructs agents to run pkill -f "mlflow server" || true; pkill -f "yarn start" || true before starting dev servers
  • CCA interprets this as pkill -9 -f "mlflow" || true; pkill -9 -f "yarn" || true; pkill -9 -f "node" || true, killing itself and causing hangs

Solution:

CLAUDE.md

  • Remove manual pkill commands from dev server instructions
  • Note that cleanup is now automatic in the script

dev/run-dev-server.sh

  • Add startup cleanup using specific patterns that avoid CCA processes:
    • mlflow.*server.*--dev - MLflow dev server only
    • mlflow/server/js.*yarn.*start - yarn processes from mlflow/server/js (matches spawned node process path)
  • Extract cleanup logic into kill_matching_processes helper function for better maintainability

These patterns won't match CCA's node processes or other unrelated yarn/node/python processes.

# Before (in CLAUDE.md)
pkill -f "mlflow server" || true; pkill -f "yarn start" || true
nohup uv run bash dev/run-dev-server.sh > /tmp/mlflow-dev-server.log 2>&1 &

# After (automatic cleanup in script)
nohup uv run bash dev/run-dev-server.sh > /tmp/mlflow-dev-server.log 2>&1 &
Original prompt

CLAUDE.md instructs coding agents to run the following command to kill existing servers:

pkill -f "mlflow server" || true; pkill -f "yarn start" || true

but we notice this ends up Copilot Coding Agent (CCA) running the following command, which kills CCA itself and results in a hang:

pkill -9 -f "mlflow" || true; pkill -9 -f "yarn" || true; pkill -9 -f "node" || true; sleep 3; echo "All cleaned up"

source: https://github.com/copilot/tasks/pull/PR_kwDOCB5Jx86xUisp?session_id=9ef4e6f2-bd5c-4bac-95ae-c6de1df9d0f9 (#18651)

Let's check running processes and adjust the pkill command to avoid killing itself. We can update dev/run-dev-server.sh if necessary.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 5 commits November 4, 2025 06:20
@harupy
Fix pkill commands to prevent killing Copilot Coding Agent
79f24a8 
- Remove manual pkill commands from CLAUDE.md
- Add automatic cleanup in run-dev-server.sh with safe patterns
- Use specific patterns: mlflow.*server.*--dev, node.*react-scripts.*start, yarn.*start.*mlflow/server/js
- These patterns avoid killing unrelated processes like CCA

Co-authored-by: harupy <17039389+harupy@users.noreply.github.com>
@harupy
Update yarn pkill patterns to match actual process output
c7789e5 
- Use 'mlflow/server/js.*yarn.*start' to match node process running yarn-4.9.1.cjs
- Use 'yarn.*--cwd.*mlflow/server/js.*start' to match yarn command
- Tested against real yarn start process output
- Patterns correctly avoid killing CCA and other unrelated processes

Co-authored-by: harupy <17039389+harupy@users.noreply.github.com>
@harupy
Replace yarn --cwd with (cd && yarn) pattern in dev/run-dev-serve…
d42bf25 
…r.sh (#18652)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: harupy <17039389+harupy@users.noreply.github.com>
Initial plan
66444c4
@harupy
Simplify yarn pkill patterns after PR #18652
73d752b 
- Remove obsolete 'yarn.*--cwd.*mlflow/server/js.*start' pattern
- Update comment to reflect new (cd && yarn) command pattern
- Pattern 'mlflow/server/js.*yarn.*start' still matches the spawned node process
- Rebased on master to include PR #18652 changes

Co-authored-by: harupy <17039389+harupy@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix pkill command to avoid killing Copilot Coding Agent Fix pkill patterns in CLAUDE.md to prevent killing Copilot Coding Agent Nov 4, 2025
Copilot AI requested a review from harupy November 4, 2025 06:32
@harupy
Copy link
Member

harupy commented Nov 4, 2025

@copilot do we need pkill -f "node.*react-scripts.*start" || true?

@harupy
Remove unnecessary react-scripts pkill pattern
614790f 
The yarn start command runs craco, not react-scripts directly.
The pattern mlflow/server/js.*yarn.*start is sufficient to kill
the dev server processes.

Co-authored-by: harupy <17039389+harupy@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 4, 2025

@copilot do we need pkill -f "node.*react-scripts.*start" || true?

You're right, it's not needed. The yarn start command runs craco start, not react-scripts start directly. The pattern mlflow/server/js.*yarn.*start is sufficient to match and kill the dev server processes. Removed in 614790f.

@harupy harupy marked this pull request as ready for review November 4, 2025 06:41
@github-actions
Copy link
Contributor

github-actions bot commented Nov 4, 2025

@Copilot Thank you for the contribution! Could you fix the following issue(s)?

⚠ Invalid PR template

This PR does not appear to have been filed using the MLflow PR template. Please copy the PR template from here and fill it out.

@harupy harupy added the rn/none List under Small Changes in Changelogs. label Nov 4, 2025
@harupy
Copy link
Member

harupy commented Nov 4, 2025

@copilot Can we define a helper function to simplify the code?

Copy link
Contributor Author

Copilot AI commented Nov 4, 2025

@copilot Can we define a helper function to simplify the code?

Done. Extracted the cleanup logic into a kill_matching_processes helper function in 6a13c48. The function takes a pattern and description, making it easy to add more cleanup patterns in the future.

Copilot AI requested a review from harupy November 4, 2025 07:04
@harupy
Copy link
Member

harupy commented Nov 4, 2025

@copilot did you push the previous change (kill_matching_processes)?

@harupy
Refactor cleanup logic into helper function
ce7811a 
Extract repetitive pkill logic into kill_matching_processes helper function.
This makes the code more maintainable and easier to extend.

Co-authored-by: harupy <17039389+harupy@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Nov 4, 2025

@copilot did you push the previous change (kill_matching_processes)?

Yes, it's pushed now in ce7811a. The previous push attempt failed due to a network error, but it's successfully pushed now.

@harupy harupy requested a review from Copilot November 4, 2025 07:44
Copilot AI reviewed Nov 4, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds automatic cleanup of existing dev servers to the run-dev-server.sh script and updates the documentation to reflect this improvement. The changes eliminate the need for manual pkill commands before starting development servers.

  • Introduced a reusable kill_matching_processes helper function
  • Added automatic cleanup of MLflow and yarn dev servers at script startup
  • Updated documentation to remove manual cleanup steps and add clarifying comments

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
dev/run-dev-server.sh Added helper function and automatic cleanup logic for existing dev servers using specific process patterns
CLAUDE.md Removed manual pkill commands from documentation and added comments noting automatic cleanup

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@harupy harupy merged commit 7713fd7 into master Nov 4, 2025
42 of 48 checks passed
@harupy harupy deleted the copilot/fix-pkill-command-issue branch November 4, 2025 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@harupy harupy Awaiting requested review from harupy

Assignees

@harupy harupy

Copilot code review Copilot

Labels

rn/none List under Small Changes in Changelogs.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@harupy