chore: rename cmd/worker → cmd/engram-server (#43)#142
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughПереименование основного двоичного файла сервера с Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request renames the core server component from "worker" to "engram-server" across build configurations, documentation, and scripts. It also updates references for the "engram" client. Feedback highlights a potential build failure in .goreleaser.yaml due to an inconsistent path for the MCP proxy and suggests optimizing the uninstallation script by removing redundant pkill commands and using SIGTERM instead of SIGKILL to allow for graceful shutdowns.
| WK["Server<br/>cmd/engram-server<br/>:37777 HTTP"] | ||
| MCP_STDIO["MCP stdio<br/>cmd/mcp"] | ||
| MCP_PROXY["MCP stdio-proxy<br/>cmd/mcp-stdio-proxy"] | ||
| MCP_PROXY["engram client<br/>cmd/engram"] |
There was a problem hiding this comment.
This change updates the reference to the MCP proxy to cmd/engram. However, .goreleaser.yaml (line 77) still points to ./cmd/mcp-stdio-proxy. Since the Dockerfile (line 39) now builds ./cmd/engram and the documentation has been updated, it is likely that the release build in .goreleaser.yaml will fail unless updated to the new path.
| pkill -9 -f 'engram-server' 2>/dev/null || true | ||
| pkill -9 -f '\.claude/plugins/.*/engram-server' 2>/dev/null || true |
There was a problem hiding this comment.
The second pkill command is redundant because the pattern 'engram-server' in the first command already matches any command line containing that string, including those in the .claude/plugins/ directory. Additionally, using SIGKILL (-9) is aggressive and prevents the application from performing a graceful shutdown (e.g., flushing logs or closing database connections). It is better to use SIGTERM (the default) first.
| pkill -9 -f 'engram-server' 2>/dev/null || true | |
| pkill -9 -f '\.claude/plugins/.*/engram-server' 2>/dev/null || true | |
| pkill -f 'engram-server' 2>/dev/null || true |
Binary naming consistency: engram (local client) + engram-server (remote server). Updated: AGENTS.md, .goreleaser.yaml, Dockerfile, docs/arch/*, scripts/uninstall.sh. internal/worker/ package NOT renamed (Go embed path, separate concern). Closes #43.
thebtf
force-pushed
the
chore/rename-server-binary
branch
from
c870f25 to
f283fe1
Compare
1 participant
Summary
git mv cmd/worker cmd/engram-serverinternal/worker/package NOT renamed (Go embed path — separate concern)Test plan
go build ./cmd/engram-server/compilesgo build ./...cleanSummary by CodeRabbit
Примечания к выпуску
Чоры (Maintenance)
Документация