fix(kimi): resolve the server port from the live lock, not a stale log record (#39)#45
Merged
Merged
Conversation
…g record `server_base_url` scraped the append-only ~/.kimi-code/server/server.log for the last `"msg":"server listening"` record and preferred it over every other source, including $C2C_KIMI_SERVER_PORT. Modern kimi-code appends that record on a COLD start only — a warm start logs plain text (`server already running (pid=…, port=…)`) — so no fresh record is ever written and the resolver ages into permanent failure, returning whatever port was bound at the last cold start. Measured on xsm: a six-day-old record named 58629 while the live server was on 58627. Result: `c2c send` reports ok, the message is never delivered, and the notifier spins on ECONNREFUSED every 2s forever against an address it never validated. Resolution precedence is now: fixture override → live lock (pid-checked) → $C2C_KIMI_SERVER_PORT → liveness-probed server.log record → default 58627 The lock (~/.kimi-code/server/lock) is kimi's own authority for the currently running server: created O_EXCL on bind, port rewritten via updatePort(), unlinked on clean shutdown, and treated as stale by kimi when its pid is dead. Kimi's experimental multi_server registry (server/instances/*.json) is read with the same semantics. The log is demoted to last-resort and TCP-probed, so a dead address is skipped instead of retried forever. Malformed or absent lock files fall through without raising. The notifier's start path now names the address it is failing against, backs off 10s → 20s → … → 5min between `kimi server run` attempts, and after three consecutive failures emits an actionable message (lock path + `ss -ltnp` recovery) rather than an identical timeout line. It stays non-fatal: undelivered mail remains in the broker inbox. Regression suite `server_base_url_stale_port_39` (9 hermetic cases, dead ports obtained by binding :0 and closing). Verified RED against the old precedence: 5 failures, headline case returning the stale port instead of the lock's. Closes #39
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
1 participant
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.
Summary
Fixes #39 — a bug that broke 100% of Kimi delivery on an affected host, silently and permanently.
server_base_url ()preferred an address scraped from the append-only~/.kimi-code/server/server.log. Modern kimi-code writes that"server listening"JSON record only on a cold start (a warm start logs plain text), so the resolver ages into permanently returning whatever port was recorded at the last cold start. Measured on this host: a 6-day-old record pointing at a dead port while the live server was elsewhere. Symptom was an endless silent loop (ECONNREFUSEDevery 2s);c2c sendreturnedokand the message was never seen.C2C_KIMI_SERVER_PORTdid not rescue it — it only fed the fallback the log-scan pre-empted.The fix — trust what's live, not what happened
New precedence, documented inline:
fixture → live lock / instance registry (pid-checked) →
$C2C_KIMI_SERVER_PORT→ liveness-probedserver.log→ default portRationale: the lock is maintained by the process actually listening and self-invalidates via pid (kimi itself treats a lock with a dead pid as takeable, so pid-liveness is the authoritative check — that's exactly what we use). Env is explicit operator intent, so it beats history. The log is a record of what happened, never of what is, so it comes last and only after a TCP probe. Only that untrusted historical candidate pays the probe cost.
Schema verified, not guessed: the lock file didn't exist on this host (kimi unlinks it on clean shutdown), so the schema was read out of kimi's own bundled binary (
acquireLock/readLockContents/instanceRegistry). This also surfaced an experimentalmulti_servermode that replaces the exclusive lock withserver/instances/<serverId>.json— read too, since a host flipping that flag would otherwise reintroduce the identical failure.Also hardened the "server not responding → start it" loop (
c2c_kimi_notifier.ml): exponential backoff (10s → 5min cap), the address named in every log line, a distinct actionable message after 3 failures, and a recovery line when it returns. Still non-fatal, mail still left queued.Verification
test_c2c_kimi_deliver25 (newserver_base_url_stale_port_39suite, 9 cases),test_c2c_kimi_notifier44,test_c2c_doctor_hooks46,test_c2c_hook_kimi6,test_c2c_setup_kimi22 — all green;dune build @checkexit 0; fork-free.RED proof: restoring only the old
server_base_urlbody (keeping the new helpers) isolates the precedence as the change under test —Dead ports are obtained by binding
:0and closing, not hard-coded.Caveat
An earlier e2e agent had already scrubbed the stale records from this host's real
server.logto unblock testing, so the fix is proven by fixture, not yet against a live warm-started kimi server. A live dogfood (restore the stale record from backup, confirm delivery still works) is the natural follow-up gate.Closes #39.