Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,27 @@ app-server, OpenCode the plugin API). That is an upstream ask, not our bug.
pid-checked) → `C2C_KIMI_SERVER_PORT` → liveness-probed `server.log` record →
default 58627. Never trust `server.log` unprobed — kimi writes the
`"server listening"` record on cold start only, so it ages into a dead port.
**The SessionStart hook is NOT the identity authority for managed sessions
(#40).** Kimi Code >= 0.27 runs sessions inside a shared, long-lived
`kimi server` daemon and spawns hook commands from *that daemon's*
environment, so `c2c hook kimi` cannot see a managed instance's
`C2C_MCP_SESSION_ID` / `C2C_MCP_AUTO_REGISTER_ALIAS`. `c2c start kimi`
therefore registers the alias itself (`register_managed_kimi_session`,
**before the fork** — the hook can fire the moment the child is up),
session_id = instance name, recording the launch cwd and the OUTER pid +
pid_start_time; the hook *adopts* that row by normalized cwd + live pid pair
instead of minting a competing alias. A failed registration is both printed
and appended to broker.log as `managed_registration_failed` — the TUI paints
over the terminal, so a terminal-only message is not "loud". Because the
launcher's sid IS the alias in the default case, it arms the notifier with
`~authoritative:true` so `decide_notifier_rekey` does not mistake the real
binding for a placeholder and strand a leftover daemon on the wrong inbox.
**Known limits:** two managed kimi instances in one directory are
indistinguishable to the hook (it bails loudly rather than guess), and a
*co-located vanilla* kimi TUI in a managed directory is adopted too — it
never registers its own alias and its identity skill names the managed alias.
Delivery is unaffected (the REST layer is workdir-keyed); nothing in the hook
payload can distinguish these cases.
Legacy notification-store runbook:
`.collab/runbooks/kimi-notification-store-delivery.md` (deprecated).
- **OpenCode**: SIGUSR1 to the *inner* OpenCode pid (not the outer wrapper)
Expand Down
33 changes: 26 additions & 7 deletions ocaml/c2c_kimi_notifier.ml
Original file line number Diff line number Diff line change
Expand Up @@ -927,18 +927,37 @@ let running_session_id alias =
placeholder is what stops a later placeholder arm (e.g. a supervisor
relaunch) from flapping a correctly-bound daemon back onto the alias.
[running = None] is a pre-#9 daemon of unknown binding: bind it as soon as
we have a real sid to bind. Pure; exposed for unit tests. *)
let decide_notifier_rekey ~alias ~requested_sid ~running_sid =
we have a real sid to bind. Pure; exposed for unit tests.

[authoritative] (#40) breaks the "sid == alias means placeholder" overload.
Since #40 the managed launcher registers session_id = the instance name and
arms the notifier on it, so in the DEFAULT managed case
(alias == name == session_id) the *authoritative* binding is byte-identical
to what this function used to treat as a placeholder. Without the flag a
leftover live notifier for the same alias bound to some other sid — after a
SIGKILLed outer loop or a failed `c2c restart` teardown — would never
converge onto <name>: it falls through to the stale-binary branch and
Skip_current's on an unchanged binary, leaving the session deaf while
`c2c send` reports success (i.e. #40's own symptom, re-introduced).
Callers that KNOW the sid is a real binding rather than a t≈0 guess pass
[~authoritative:true]; the placeholder guard is then skipped and only the
"differs from what is running" test applies. Default [false] preserves the
pre-#40 behaviour for every other caller. *)
let decide_notifier_rekey ~alias ~requested_sid ?(authoritative = false)
~running_sid () =
(* Placeholder compare is case-insensitive, matching how aliases are
compared everywhere else (alias comparisons are case-insensitive per
B112, and pick_live_registration_sid matches the same way). *)
if String.lowercase_ascii requested_sid = String.lowercase_ascii alias then
false
if
(not authoritative)
&& String.lowercase_ascii requested_sid = String.lowercase_ascii alias
then false
else match running_sid with
| None -> true
| Some cur -> cur <> requested_sid

let ensure_daemon ~alias ~broker_root ~session_id ~tmux_pane ?(interval = 2.0) () =
let ensure_daemon ~alias ~broker_root ~session_id ?(authoritative = false)
~tmux_pane ?(interval = 2.0) () =
let pidfile = pidfile_path alias in
(* Identity gate FIRST (B145 PID-reuse guard). Treat the pidfile pid as the
running notifier ONLY if it is alive AND comm-matches. A dead pid OR a
Expand All @@ -958,8 +977,8 @@ let ensure_daemon ~alias ~broker_root ~session_id ~tmux_pane ?(interval = 2.0) (
(* nothing of ours running (incl. a just-cleaned stale pidfile) → fresh *)
start_daemon ~alias ~broker_root ~session_id ~tmux_pane ~interval ()
| Some pid when
decide_notifier_rekey ~alias ~requested_sid:session_id
~running_sid:(running_session_id alias) ->
decide_notifier_rekey ~alias ~requested_sid:session_id ~authoritative
~running_sid:(running_session_id alias) () ->
(* #9 B: live daemon of ours, but bound to the WRONG session_id — it is
draining an inbox no mail lands in. Re-key by cycling it onto the
requested sid. [pid] is a CONFIRMED-ours notifier (identity-gated
Expand Down
28 changes: 25 additions & 3 deletions ocaml/c2c_kimi_notifier.mli
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,25 @@ val running_session_id : string -> string option
Re-keys iff [requested_sid] differs from what is running AND is not the
alias placeholder — refusing to downgrade back to the placeholder is what
prevents a later placeholder arm from flapping a correctly-bound daemon.
Pure; exposed for unit tests. *)
Pure; exposed for unit tests.

[authoritative] (#40) opts out of the placeholder guard. Since #40 the
managed launcher registers [session_id = <instance name>] and arms the
notifier on it, so in the default managed case
([alias = name = session_id]) an AUTHORITATIVE binding is byte-identical to
a placeholder. Without this flag a leftover live notifier bound to a
different sid (SIGKILLed outer loop, failed `c2c restart` teardown) never
converges onto [<name>] — it reaches the stale-binary branch and
[Skip_current]s on an unchanged binary, leaving the session deaf while
`c2c send` reports success. Pass [true] only when the sid is a real
binding rather than a t≈0 guess. Defaults to [false]. *)
val decide_notifier_rekey :
alias:string -> requested_sid:string -> running_sid:string option -> bool
alias:string ->
requested_sid:string ->
?authoritative:bool ->
running_sid:string option ->
unit ->
bool

(** [already_running alias] returns [true] iff the notifier pidfile for
[alias] names a process that is alive AND whose comm matches the daemon's
Expand Down Expand Up @@ -125,11 +141,17 @@ val decide_notifier_start :

SHA sources can be overridden for tests via
[C2C_KIMI_NOTIFIER_FIXTURE_RUNNING_SHA] /
[C2C_KIMI_NOTIFIER_FIXTURE_INSTALLED_SHA]. *)
[C2C_KIMI_NOTIFIER_FIXTURE_INSTALLED_SHA].

[authoritative] is forwarded to {!decide_notifier_rekey}: pass [true] when
[session_id] is a known-real binding (the managed launcher's own
registration) rather than a t≈0 placeholder guess, so a leftover daemon
bound elsewhere is re-keyed onto it (#40). *)
val ensure_daemon :
alias:string ->
broker_root:string ->
session_id:string ->
?authoritative:bool ->
tmux_pane:string option ->
?interval:float ->
unit ->
Expand Down
Loading
Loading