fix(cli): deps field, root mismatch, add --help/--version#91
Conversation
Benchmark Regression ReportThreshold: 10.00%
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ef6b6e9d64
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| running_root=$(curl -sf "$BASE/health" | jq -r '.root // empty' 2>/dev/null) | ||
| if [ -n "$running_root" ] && [ "$running_root" != "$root" ]; then |
There was a problem hiding this comment.
This root-mismatch safeguard never activates because it parses .root from /health, but the server currently responds with only {"status":"ok"} (see src/server.zig health handler at lines 106-108). In that common case running_root is empty, so the code always takes the return 0 path and still serves queries from the wrong repository when a daemon is already running on another root.
Useful? React with 👍 / 👎.
justrach
force-pushed
the
fix/cli-bugs-90
branch
from
ef6b6e9 to
28a6f78
Compare
|
Fixed Codex's P1 finding: Root mismatch check now uses process args instead of The # Before (broken): parsed .root from /health (always empty)
running_root=$(curl -sf "$BASE/health" | jq -r '.root // empty')
# After (fixed): parse from process command line
running_root=$(ps -eo args | grep "[c]odedb.*serve" | awk '{for(i=1;i<=NF;i++) if($i=="serve") print $(i-1)}')@codex review the updated fix. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28a6f78312
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Check if the running daemon is serving the requested root | ||
| # Parse root from the running codedb process's command line | ||
| local running_root | ||
| running_root=$(ps -eo args 2>/dev/null | grep "[c]odedb.*serve" | awk '{for(i=1;i<=NF;i++) if($i=="serve") print $(i-1)}' | head -1) |
There was a problem hiding this comment.
The root-detection pipeline hardcodes grep "[c]odedb.*serve" while the CLI explicitly supports custom executables via CODEDB_BINARY. Under set -euo pipefail, a healthy daemon started from a differently named binary (or wrapper) makes this grep return 1, which aborts _ensure_server at assignment time instead of returning success after _ping. This turns normal commands into immediate failures whenever CODEDB_BINARY does not contain codedb.
Useful? React with 👍 / 👎.
| running_root=$(ps -eo args 2>/dev/null | grep "[c]odedb.*serve" | awk '{for(i=1;i<=NF;i++) if($i=="serve") print $(i-1)}' | head -1) | ||
| if [ -n "$running_root" ]; then | ||
| running_root="$(cd "$running_root" 2>/dev/null && pwd || echo "$running_root")" | ||
| fi | ||
| if [ -n "$running_root" ] && [ "$running_root" != "$root" ]; then |
There was a problem hiding this comment.
After _ping confirms only the daemon on $CODEDB_PORT, the code still reads running_root from the first process matching codedb.*serve (head -1) without tying it to that port. In environments with multiple codedb daemons, this can compare against another instance’s root, falsely detect a mismatch, and trigger pkill -f "codedb.*serve", interrupting unrelated daemons.
Useful? React with 👍 / 👎.
justrach
force-pushed
the
fix/cli-bugs-90
branch
from
28a6f78 to
4947e7d
Compare
|
Addressed both Codex findings: P1 (binary name): No longer greps for hardcoded P2 (multi-daemon): Identifies daemon by port first, only kills that specific PID (not # Find PID of daemon on our port
daemon_pid=$(ss -tlnp "sport = :$PORT" | awk '/LISTEN/{gsub(/.*pid=/,""); gsub(/,.*/,""); print; exit}')
# Read its cmdline to extract root
running_root=$(tr '\0' ' ' < "/proc/$daemon_pid/cmdline" | awk '{for(i=1;i<=NF;i++) if($i=="serve") print $(i-1)}')
# Kill only this specific daemon if root mismatches
kill "$daemon_pid"@codex review |
Benchmark Regression ReportThreshold: 10.00%
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4947e7d2fb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fi | ||
| if [ -n "$daemon_pid" ]; then | ||
| # Read cmdline: tokens are null-separated in /proc | ||
| running_root=$(tr '\0' ' ' < "/proc/$daemon_pid/cmdline" 2>/dev/null | awk '{for(i=1;i<=NF;i++) if($i=="serve") print $(i-1)}') |
There was a problem hiding this comment.
running_root is extracted from /proc/$daemon_pid/cmdline by converting NULs to spaces and then using default AWK field splitting. This breaks when the served root contains spaces (for example /tmp/my repo): the script captures only the last word before serve, so _ensure_server thinks the root changed and kills/restarts the daemon on every command even when it is already serving the requested repo. Parse the NUL-delimited argv directly instead of space-splitting to avoid false mismatches.
Useful? React with 👍 / 👎.
Fixes #90 (found by Codex review on PR #88): 1. deps: use `.imported_by[]` not `.dependents[]` to match server response 2. _ensure_server: check running daemon's root via /health, restart if it doesn't match the requested root (prevents silent wrong-repo results) 3. Add --help/-h and --version/-v flags Generated with AI Co-Authored-By: AI <ai@example.com>
justrach
force-pushed
the
fix/cli-bugs-90
branch
from
4947e7d to
c200fdf
Compare
Benchmark Regression ReportThreshold: 10.00%
|
Changes since 0.2.0: - fix: stack overflow on Linux (64MB thread trampoline) (#88, closes #84) - fix: conditional codesign (macOS only) (#88) - fix(cli): deps JSON field, root mismatch, --help/--version (#91, closes #90) - feat: CLI wrapper (codedb-cli) + systemd service + docs Generated with AI Co-Authored-By: AI <ai@example.com>
1 participant
Fixes #90
Changes
Bug 1:
depsuses wrong JSON fieldjq '.dependents[]'→jq '.imported_by[]'to match server response (src/server.zigline 481).Bug 2: Root mismatch when daemon already running
_ensure_servernow checks the running daemon's root via/healthresponse. If a different root is requested, it restarts the daemon instead of silently returning results from the wrong repo.New:
--help/--versionflagsOrigin
Both bugs found by Codex automated review on #88.
@codex review