Skip to content

Advertise the instance from enableRemote and report attention status on the heartbeat - #12532

Merged
iscekic merged 3 commits into
mainfrom
fix/cli-remote-advertise-and-attention-status
Jul 25, 2026
Merged

Advertise the instance from enableRemote and report attention status on the heartbeat#12532
iscekic merged 3 commits into
mainfrom
fix/cli-remote-advertise-and-attention-status

Conversation

@iscekic

@iscekic iscekic commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

Two CLI-side fixes for defects found by an end-to-end verification run of the Kilo mobile app, each confirmed by an independent code triage before any code was written.

/remote never registered the CLI as a spawn target

Enabling the remote relay from the TUI /remote slash command connected the socket and mirrored sessions correctly, but the CLI never appeared in the mobile app's "Run on" picker. Only the explicit kilo remote command called setInstanceAdvertisement, so getConnectedInstances() filtered the socket out on the cloud side.

The advertisement now runs on every successful enableRemote() entry. The placement matters more than it looks: enableRemote() opens with if (remote) return, and bootstrap auto-enable frequently connects first, so /remote usually hits that early return — an advertisement placed in the connection-setup body would have left the defect unfixed in the common case. ingestDisabled returns before the advertisement and stays unadvertised.

The ensure helper no-ops when an advertisement is already set, so it fires no extra heartbeat, while explicit setInstanceAdvertisement keeps its existing replace semantics and tests. buildInstanceAdvertisement moved to a shared module so both entry points derive it identically.

A session blocked on a question reported as busy

The heartbeat built each session's status from SessionStatus.Service — union idle|retry|busy|offline — which never consults Question.Service or Permission.Service. deriveStatus() already did consult both, but only fed the ingest session_status sync. So a session genuinely blocked on a question was advertised as busy, and the mobile app, which takes live row status from the heartbeat, showed no needs-input badge.

The precedence (permission, then question, then SessionStatus) is now a shared helper used by both deriveStatus and the heartbeat, so the two channels cannot drift.

The heartbeat runs on a ~10s timer across every session and deriveStatus makes service calls per session, so the permission and question lists are fetched once per tick and indexed by session id rather than queried per session. A test pins the call count.

Behaviour note

Sharing the derivation also means a SessionStatus of offline now reports as retry on the wire, matching what deriveStatus has always sent to ingest. Nothing consumes offline from the heartbeat — the transport forwards only idle and busy, and the mobile row treats both as non-attention — so the practical effect is that the two channels now agree. The detach fence test is parameterised accordingly; its assertion that the status clears on detach is unchanged.

Relationship to the cloud change

Kilo-Org/cloud#4769 fixes the same needs-input defect from the other side, by overlaying the stored status onto live rows. That change covers already-released CLIs and does not depend on this one; this change fixes it at the source so the two status channels agree. Neither PR blocks the other.

Checks

packages/opencode typecheck and the affected suites pass (21 tests including the DEF-1 contract cases and the DEF-3 batching call-count assertion). Independent reviewer over the full diff: no findings.

iscekic added 2 commits July 25, 2026 13:15
…rs as a spawn target

Enabling the remote relay from the TUI `/remote` slash command connected the
socket and mirrored sessions, but never advertised the instance, so the CLI
never appeared as a spawn target in the mobile "Run on" picker. Only the
explicit `kilo remote` command called setInstanceAdvertisement.

The advertisement now runs on every successful enableRemote() entry, before the
already-connected and coalescing early returns. That ordering matters: bootstrap
auto-enable frequently connects first, so `/remote` usually hits
`if (remote) return` and an advertisement placed in the connection-setup body
would leave the defect unfixed in the common case. `ingestDisabled` returns
before the advertisement and stays unadvertised.

The ensure helper is a no-op when an advertisement is already set, so it fires no
extra heartbeat, while explicit setInstanceAdvertisement keeps its existing
replace semantics. buildInstanceAdvertisement moves to a shared module so the
command path and the enable path derive it identically.
…beat

The heartbeat built each session's status from SessionStatus.Service, whose
union is idle/retry/busy/offline and which never consults Question.Service or
Permission.Service. deriveStatus() already did consult both, but only fed the
ingest session_status sync. So a session genuinely blocked on a question was
advertised as busy on the heartbeat, and the mobile app — which takes live row
status from the heartbeat — showed no needs-input badge.

Extract the precedence (permission, then question, then SessionStatus) into a
shared helper used by both deriveStatus and the heartbeat, so the two channels
cannot drift.

The heartbeat runs on a ~10s timer across every session, and deriveStatus makes
service calls per session, so the permission and question lists are fetched once
per tick and indexed by session id rather than queried per session. A test pins
the call count.

Behaviour note beyond the strict fix: sharing the derivation also means a
SessionStatus of offline now reports as retry on the wire, matching what
deriveStatus has always sent to ingest. Nothing consumes offline from the
heartbeat — the transport forwards only idle and busy, and the mobile row treats
both as non-attention — so the effect is that the two channels now agree. The
detach fence test is parameterised accordingly; its assertion that the status
clears on detach is unchanged.
@iscekic iscekic self-assigned this Jul 25, 2026
…ntion tests

The DEF-3 heartbeat tests raise and reply to real Question and Permission
requests through the global AppRuntime, which took kilo-sessions.test.ts from 4
classified references to 29 and failed the allowlist check.

Bumping the count rather than restructuring the tests is deliberate: the
heartbeat resolves attention status from the global Question.Service and
Permission.Service, so asserting it requires driving those same services.
Scoped layers cannot express that — the global-runtime coupling is the thing
under test — and it is the same integration pattern this entry already
sanctioned for the detach fence. The reason string records that.
@iscekic

iscekic commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator Author

(bot) Device E2E complete on e76b352689. Both CLI-side fixes verified against a local stack, using a build of this branch (0.0.0-fix-cli-remote-advertise-and-attention-status-…) alongside the stock released 7.4.16 as a control.

DEF-1 — /remote registers the CLI as a spawn target: PASS. With remote enabled from the TUI slash command, the instance appears in the mobile app's "Run on" picker, and a session started from the app reaches the CLI and returns a real response. The control confirms the change is what did it: the stock CLI, with /remote enabled the same way and its sessions mirroring correctly, does not appear in the picker.

DEF-3 — heartbeat reports pending attention: PASS.

Sample Live status on the raw worker endpoint
Patched, question pending, +31s question
Patched, +12s later question (stable)
Patched, after answering leaves questionidle
Stock 7.4.16, question pending, 3 samples over ~25s busy, busy, busy

One correction to an earlier interim result. A first E2E pass reported this as "not proven" because the live endpoint still showed busy. That was a sampling race, not a defect: the DB flips within milliseconds of the question being raised, but the heartbeat only fires every ~10s, so a request issued immediately after can still return the previous heartbeat. Re-sampled after a full heartbeat period, the patched build reports question correctly. No code change was needed, and none was made — the investigation is recorded here because "we changed nothing and it now passes" deserves the evidence.

The stock column is also why Kilo-Org/cloud#4769 exists: released CLIs will never report attention on the heartbeat, so the cloud side overlays the stored status. That PR is independent of this one and needs no coordination to land.

@iscekic
iscekic enabled auto-merge (squash) July 25, 2026 13:28
@iscekic
iscekic merged commit a19d44c into main Jul 25, 2026
29 checks passed
@iscekic
iscekic deleted the fix/cli-remote-advertise-and-attention-status branch July 25, 2026 13:45
unixcrh pushed a commit to unixcrh/kilocode that referenced this pull request Aug 1, 2026
* fix(tui): prevent home wordmark corruption in height-constrained terminals (Kilo-Org#13069)

* feat(prompt): mode-specific input placeholders (Kilo-Org#12388)

* fix(tui): keep /share available to copy existing link (Kilo-Org#12532)

* fix(tui): dismiss dialogs with ctrl+c (Kilo-Org#12884)

* fix(app): terminal resize

* fix(console): translations

* fix(app): terminal PTY buffer carryover

* fix(app): notifications on child sessions

* Revert "feat(desktop): add WSL backend mode (Kilo-Org#12914)"

This reverts commit 213a872.

* release: v1.1.58

* refactor: kilo compat for v1.1.58

---------

Co-authored-by: Kit Langton <kit.langton@gmail.com>
Co-authored-by: Adam <2363879+adamdotdevin@users.noreply.github.com>
Co-authored-by: opencode <opencode@sst.dev>
unixcrh pushed a commit to unixcrh/kilocode that referenced this pull request Aug 1, 2026
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
* fix(tui): prevent home wordmark corruption in height-constrained terminals (Kilo-Org#13069)

* feat(prompt): mode-specific input placeholders (Kilo-Org#12388)

* fix(tui): keep /share available to copy existing link (Kilo-Org#12532)

* fix(tui): dismiss dialogs with ctrl+c (Kilo-Org#12884)

* fix(app): terminal resize

* fix(console): translations

* fix(app): terminal PTY buffer carryover

* fix(app): notifications on child sessions

* Revert "feat(desktop): add WSL backend mode (Kilo-Org#12914)"

This reverts commit e63699f.

* release: v1.1.58

* refactor: kilo compat for v1.1.58

---------

Co-authored-by: Kit Langton <kit.langton@gmail.com>
Co-authored-by: Adam <2363879+adamdotdevin@users.noreply.github.com>
Co-authored-by: opencode <opencode@sst.dev>
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
…on the heartbeat (Kilo-Org#12532)

* fix(cli): advertise the instance from enableRemote so /remote registers as a spawn target

Enabling the remote relay from the TUI `/remote` slash command connected the
socket and mirrored sessions, but never advertised the instance, so the CLI
never appeared as a spawn target in the mobile "Run on" picker. Only the
explicit `kilo remote` command called setInstanceAdvertisement.

The advertisement now runs on every successful enableRemote() entry, before the
already-connected and coalescing early returns. That ordering matters: bootstrap
auto-enable frequently connects first, so `/remote` usually hits
`if (remote) return` and an advertisement placed in the connection-setup body
would leave the defect unfixed in the common case. `ingestDisabled` returns
before the advertisement and stays unadvertised.

The ensure helper is a no-op when an advertisement is already set, so it fires no
extra heartbeat, while explicit setInstanceAdvertisement keeps its existing
replace semantics. buildInstanceAdvertisement moves to a shared module so the
command path and the enable path derive it identically.

* fix(cli): report pending question and permission on the session heartbeat

The heartbeat built each session's status from SessionStatus.Service, whose
union is idle/retry/busy/offline and which never consults Question.Service or
Permission.Service. deriveStatus() already did consult both, but only fed the
ingest session_status sync. So a session genuinely blocked on a question was
advertised as busy on the heartbeat, and the mobile app — which takes live row
status from the heartbeat — showed no needs-input badge.

Extract the precedence (permission, then question, then SessionStatus) into a
shared helper used by both deriveStatus and the heartbeat, so the two channels
cannot drift.

The heartbeat runs on a ~10s timer across every session, and deriveStatus makes
service calls per session, so the permission and question lists are fetched once
per tick and indexed by session id rather than queried per session. A test pins
the call count.

Behaviour note beyond the strict fix: sharing the derivation also means a
SessionStatus of offline now reports as retry on the wire, matching what
deriveStatus has always sent to ingest. Nothing consumes offline from the
heartbeat — the transport forwards only idle and busy, and the mobile row treats
both as non-attention — so the effect is that the two channels now agree. The
detach fence test is parameterised accordingly; its assertion that the status
clears on detach is unchanged.

* chore(cli): widen the promise-facade allowlist for the heartbeat attention tests

The DEF-3 heartbeat tests raise and reply to real Question and Permission
requests through the global AppRuntime, which took kilo-sessions.test.ts from 4
classified references to 29 and failed the allowlist check.

Bumping the count rather than restructuring the tests is deliberate: the
heartbeat resolves attention status from the global Question.Service and
Permission.Service, so asserting it requires driving those same services.
Scoped layers cannot express that — the global-runtime coupling is the thing
under test — and it is the same integration pattern this entry already
sanctioned for the detach fence. The reason string records that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants