Harden backend and dashboard startup races#119
Merged
yyiilluu merged 6 commits intoJul 9, 2026
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR replaces fixed Reflexio and service ports with environment-driven defaults, adds shared locking and port/process helpers, refactors backend and dashboard lifecycle scripts around those helpers, and makes the Codex hook delegate service startup to shell scripts. ChangesConfigurable ports and service lifecycle refactor
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugin/scripts/_lib.sh`:
- Around line 810-869: The command lookup in claude_smart_pid_command can
truncate long process arguments, which can prevent claude_smart_port_holder and
the related port-listener matching flow from identifying stale listeners
correctly. Update the non-Windows ps invocation to use the wider output form so
the full command line is preserved, keeping the existing fallback behavior and
the surrounding claude_smart_port_listener_pids and claude_smart_port_holder
logic unchanged.
In `@plugin/scripts/backend-service.sh`:
- Around line 169-172: The current cold-start skip in
recorded_backend_pid_alive/start can suppress backend restarts indefinitely if
the recorded PID stays alive but never becomes healthy. Update the start flow to
keep the existing PID check, but only honor the skip within a bounded startup
grace window, then fall through to the existing port check and reap/restart path
afterward. Use the recorded_backend_pid_alive helper and the start logic around
the existing skip/health decision to locate the fix, and make sure unhealthy or
stale-but-alive PIDs no longer prevent later starts forever.
In `@plugin/scripts/codex-hook.js`:
- Around line 13-15: The port defaults in codex-hook.js are accepting any env
value via Number(), which can produce NaN and propagate invalid BACKEND_PORT,
EMBEDDING_PORT, and DASHBOARD_PORT values into the generated service config.
Update the parsing around DEFAULT_BACKEND_PORT, DEFAULT_EMBEDDING_PORT, and
DASHBOARD_PORT to validate each env-derived value is an integer TCP port before
using it, and fall back to the hardcoded defaults otherwise. Keep the fix
localized to the port initialization logic so the downstream script generation
only serializes valid ports.
- Around line 401-421: The delegated script execution in codex-hook’s hook
runner can block indefinitely because spawnSync has no timeout. Update the
spawnSync call to use a hard timeout for the script invocation, then check
result.error/result.signal and append the error details to the log before
returning a safe non-failure response. Keep the fix localized around the
spawnSync/appendLog/emitNormalizedHookOutput flow in codex-hook.js so
SessionStart cannot hang if the delegated service script stalls.
In `@plugin/scripts/dashboard-service.sh`:
- Around line 64-68: The stop_dashboard_listener flow can still kill an
unrelated process because marker_responds and the port reap are separate steps
and the wildcard pattern is too broad. Update stop_dashboard_listener to target
the recorded dashboard PID and only fall back to narrow dashboard/Next command
matches in claude_smart_reap_port_listeners_matching, keeping the guard tied to
the actual listener identity rather than any process on $PORT.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 973e5b1f-452b-4746-8241-e9f2ee02cfd4
📒 Files selected for processing (15)
plugin/dashboard/app/api/reflexio/[...path]/route.tsplugin/dashboard/lib/config-file.tsplugin/scripts/_lib.shplugin/scripts/backend-service.shplugin/scripts/codex-hook.jsplugin/scripts/dashboard-open.shplugin/scripts/dashboard-service.shplugin/scripts/hook_entry.shplugin/src/claude_smart/reflexio_adapter.pytests/conftest.pytests/integration/integration.shtests/test_adapter.pytests/test_codex_support.pytests/test_dashboard_managed_reflexio.pytests/test_install_scripts.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_install_scripts.py`:
- Around line 2226-2231: The port selection in the test setup is reusing an
ephemeral port because the first socket is closed before the second bind; keep
both temporary sockets open while assigning cold_backend_port and
cold_embedding_port so they cannot match. Update the socket handling in the test
around the listener binds to preserve both reservations until both ports are
captured, ensuring the curl stub routes backend and embedding health checks
separately.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 901bd661-20c3-4cc1-aa71-0a446faac78c
📒 Files selected for processing (6)
plugin/scripts/_lib.shplugin/scripts/backend-service.shplugin/scripts/codex-hook.jsplugin/scripts/dashboard-service.shtests/test_codex_support.pytests/test_install_scripts.py
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/test_codex_support.py
- plugin/scripts/codex-hook.js
- plugin/scripts/_lib.sh
- plugin/scripts/backend-service.sh
- plugin/scripts/dashboard-service.sh
wenchanghan
force-pushed
the
codex/service-start-race-hardening
branch
from
July 8, 2026 06:37
bec68ee to
4fd15cd
Compare
wenchanghan
force-pushed
the
codex/service-start-race-hardening
branch
from
July 9, 2026 03:07
16b6ba9 to
2ab1096
Compare
wenchanghan
added a commit
to wenchanghan/claude-smart
that referenced
this pull request
Jul 14, 2026
….txt The dashboard could silently die (a crash, or a macOS jetsam pressure-kill of next-server) with no supervisor, staying down until the next *real* SessionStart hook happened to run `start` again — observed as a ~41-minute outage. Run next-server under a new dashboard-supervise.sh that respawns it within seconds, with a crash-loop guard (bounded consecutive fast failures) and a TERM/INT trap so an intentional `stop` (process-group kill) tears it down without respawning. The now-redundant retry-once block in dashboard-service.sh is removed: the supervisor owns retries, and re-spawning there would start a second supervisor racing for the port. The supervisor also does not fight an intentional stop: a child that exits via SIGTERM (143) — the port reaper in `stop`/reinstall — makes it exit rather than resurrect the dashboard; only crashes and jetsam SIGKILLs respawn. Symmetric in spirit to the backend gap in ReflexioAI#108; the flap predecessor ReflexioAI#111 (EADDRINUSE) was fixed in ReflexioAI#119, and coexistence there is kept. Also fix plugin-root marker drift: on macOS/Linux ensure-plugin-root.sh only ever wrote the symlink, while plugin-root.txt was written by the installer at a different time — so after a version change the file could point at a stale (even deleted) root while the symlink was correct. plugin-root.txt is the fallback consulted by OpenCode's server.mts and by Windows junction consumers. Write it on every symlink rewrite and reconcile it from the link on the no-rewrite exits. Verified: full install-scripts suite green; new tests cover the respawn crash-loop guard and the marker reconcile; live-tested that killing next-server respawns it within ~2s and that a process-group TERM stops the supervisor with no respawn. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
2 participants
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.
What Was Annoying
claude-smart can be started by Claude Code, Codex, OpenCode, install/update flows, and old plugin cache roots. Those callers could race each other, misread a healthy service as stale, or kill/restart a backend that another compatible claude-smart root was already using.
This PR fixes #111 by making claude-smart converge on one safe local backend/dashboard without depending on Reflexio CLI changes.
What Changed
~/.claude-smart/backend.pidonly as guarded migration input while writing per-root backend PID files going forward.BACKEND_PORT,EMBEDDING_PORT, andDASHBOARD_PORTthrough hooks, scripts, adapters, dashboard config, and tests.How It Works
flowchart TD A["Host starts claude-smart"] --> B["backend-service.sh"] B --> C{"Backend port listener?"} C -- "No" --> D["Start current backend"] C -- "Foreign" --> E["Report conflict\nDo not kill"] C -- "claude-smart" --> F{"Version/root check"} F -- "current / same / newer / unknown" --> G["Reuse existing backend"] F -- "strictly older" --> H["Reap stale backend"] H --> D D --> I["Return continue"] E --> I G --> IThe backend still calls the normal Reflexio start command after claude-smart has made its own ownership decision:
reflexio services start --only backend --no-reload --workers 1Compatibility Notes
x-claude-smart-*identity headers are intentionally no longer the source of truth. Backend identity now comes from managed process metadata; operational checks should usebackend-service.sh statusor process inspection. The dashboard still exposes its own marker because it owns that route directly.backend-python-runner.pyis part of the managed backend start contract. It runs through the prepared plugin venv and keeps claude-smart ownership/version/vendor metadata visible in argv for later stop/restart decisions.~/.claude-smart/backend.pidfrom another cache root may remain when ownership cannot be proven. That is intentional: listener classification controls startup, and leaving an unowned shared PID is safer than cross-root killing.How It Was Verified
bash -n plugin/scripts/backend-service.sh plugin/scripts/dashboard-service.sh plugin/scripts/_lib.sh plugin/scripts/backend-log-runner.sh plugin/scripts/hook_entry.sh plugin/scripts/dashboard-open.sh tests/integration/integration.shuv run --project plugin --with pytest pytest tests/test_install_scripts.py -k 'backend_service or backend_start or backend_stop or service_lock or dashboard_service_accepts_healthy_shared_dashboard_marker' -q-> 24 passeduv run --project plugin --with pytest pytest tests/test_install_scripts.py -q-> 135 passed, 1 warninguv run --project plugin --with pytest pytest tests -q-> 637 passed, 9 warningsuvx ruff check plugin/scripts/backend-python-runner.py tests/test_install_scripts.py-> passedpython3 -m py_compile plugin/scripts/backend-python-runner.pynode --check plugin/scripts/codex-hook.jsgit diff --checkRisks / Follow-ups