feat(opencode): remote create_session fields, org metadata, rename/title sync, cancel proof

[iscekic]

Issue

N/A — workflow section remote-cli-lifecycle-0b3a (no tracking issue; exception per checklist). Cloud sibling PR: Kilo-Org/cloud#4894

Context

Remote CLI session lifecycle, CLI side. The cloud platform can now spawn remote CLI sessions with an explicit agent/model and org attribution, renames sync in both directions, and auto-titles no longer get stuck behind the creation-time default title. /exit was verified end to end (verify-first; strong existing coverage plus one new locking test), and cancel→reprompt is proven at the SessionPrompt/Runner level (no hang found — no production change needed).

Implementation

Wire extension (K1). create_session keeps protocolVersion: 1 and gains optional agent: string, model: { providerID, modelID, variant? }, orgId: uuid on the existing strict schema — unknown fields still reject with invalid create_session command, which is exactly what an old CLI answers to a new client (the cloud SDK retries once with bare {protocolVersion: 1}; degradation is honest, no version bump, no relay change). The production session.create default now forwards {agent, model, metadata} into Session.Service.create (previously Record<string, never>).

Org attribution. The wire orgId rides the session's own metadata: { orgId } at create — the claim is in the row before Session.Event.Created fires, so the first kilo_meta always carries it (no create-order race, no shared-file edits). meta() resolves per-session info.metadata.orgId → KILO_ORG_ID env → auth accountId, and falls through on a get failure rather than throwing. The server re-checks membership at ingest (honesty gate); the CLI merely claims.

Rename sync. Inbound: system session.renamed is parsed (zod), applied via Session.Service.setTitle, and recorded in a new leaf module rename-adoptions.ts (markRenameAdopted/consumeRenameAdoption, markAutoTitle/consumeAutoTitle) so the title watcher never echoes an adopted rename back. Outbound: the existing Session.Event.Updated watch now tracks last-known titles (seeded on Created and at bootstrap so the first real change is never consumed as a seed) and POSTs every non-adopted title change to the ingest title route with a generated flag — ensureTitle marks its auto-titles before setTitle (clear-on-failure), everything else reports generated: false. Same-title Updated events still consume a pending adoption (the backend re-emits until heartbeats match), so a mark can never stick and swallow a later local rename. The backend is generation-aware: generated: true applies only over NULL/default titles, explicit renames are last-write-wins.

Cancel proof (K2). New SessionPrompt-level tests prove cancel-when-idle, cancel mid-stream, cancel mid-tool, cancel with a queued follow-up (deterministic queue wait), and cancel→reprompt after abortIntakes all settle to idle and a fresh prompt completes. No hang or stuck-busy reproduced → no production change; the E2E stop→send case is the remaining acceptance gate.

Tradeoffs / notes. ensureTitle (shared opencode file) got the section's only edit — the auto-title mark and an apply-time re-check, under kilocode_change markers. A picker change that never reached a send or run is invisible to the SDK by design (mode travels per message); inheritance uses the mode the session last ran with.

Item 8 — /exit verification

Verdict: verified-correct-with-evidence on every CLI hop; one locking test added. Full trace (mobile/SDK/relay hops are verified in the cloud PR):

Hop	Evidence	Verdict
CLI detach + heartbeat negative-containment fence	kilo-sessions.ts detach fence; attached-state.test.ts; remote-ws AC6f	verified-correct-with-evidence
remaining-count (remaining>0 → no exit callback; =0+callback → ACK then callback; =0+no-callback → host alive)	remote-sender.ts:770-782 + existing tests (remote-sender.test.ts:2618-3588, remote-exit.test.ts)	verified-correct-with-evidence
TUI callback (interactive)	remote-exit-worker.ts register-after-tuiReady + replacement lifecycle; worker-shutdown.test.ts drain ordering	verified-correct-with-evidence
Host survival (headless)	exit_cli keeps the headless host alive… in remote-sender tests	verified-correct-with-evidence
Survivor session after sibling exit	was implicit only — new survivor session keeps accepting send_message after sibling exit_cli in remote-sender.test.ts	gap-found-test-added

No broken behavior found → no fix.

Screenshots / Video

N/A — no visual changes (CLI/TUI surfaces unchanged; the mobile/web visuals are in the cloud PR).

How to Test

Manual/local verification

• cd packages/opencode && bun run typecheck — green (agent-run)
• bun test ./test/kilocode/sessions/remote-sender.test.ts ./test/kilocode/sessions/kilo-sessions-title.test.ts ./test/kilocode/sessions/rename-adoptions.test.ts ./test/kilocode/sessions/ensure-title-mark.test.ts — 106 pass (agent-run)
• bun test ./test/session/prompt.test.ts -t "cancel" — 16 pass (cancel→reprompt proofs, run multiple times for stability)
• bun test ./test/effect/runner.test.ts — 25 pass
• bun run script/check-opencode-annotations.ts --worktree — clean (shared-file edits carry kilocode_change markers)
• bun run lint (repo root) — 0 errors (warnings pre-existing)

Reviewer test steps

1. bun test ./test/kilocode/sessions/remote-sender.test.ts -t "create_session" — wire extension accepts agent/model/orgId; unknown fields still reject
2. bun test ./test/kilocode/sessions/kilo-sessions-title.test.ts — org precedence (metadata > env > auth), generation-aware title broadcast, adoption suppression, same-title mark consumption
3. bun test ./test/session/prompt.test.ts -t "cancel drops a queued" — cancel drops a genuinely queued follow-up (deterministic wait), then reprompt completes

Blocked checks and substitute verification

• Full bun test ./test/session/prompt.test.ts shows one pre-existing failure — shell correlates the persisted tool part with its completed v2 record — which also fails on the base commit (verified by checking out the pre-change file version), and two load-sensitive tests that pass in isolation. Not caused by this PR; the 5 new cancel tests and all touched suites pass.

Checklist

• Issue linked above, or exception explained
• Tests/verification described
• Screenshots/video included for visual changes, or marked N/A
• Changeset considered for user-facing changes — .changeset/remote-cli-lifecycle.md (minor @kilocode/cli)
• I personally reviewed the diff and can explain the changes, including any AI-assisted work.

Get in Touch

iscekic (assignee) is on the Kilo Code Discord.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Incremental review of 284d7e85..94a04fdd — a single test-only line change that raises the per-test timeout for returns false when an active prompt wins the deletion race from 10_000 to 30_000 with a kilocode_change note explaining the Windows CI headroom. The rationale holds up: the slow steps in that test (tmpdirScoped({ git: true }), provider config, spawning the test LLM server, and llm.wait(1)) have no inner timeout of their own, so the outer test timeout is the only guard and 10s is tight on a slow Windows runner. Real assertions are unchanged, and the inner pollWithTimeout still fails fast at its 5s default with a descriptive message, so the raised ceiling does not mask a hang in the behavior under test. No production code and no new resource surface.

Files Reviewed (1 file in this increment)

• packages/opencode/test/server/httpapi-session.test.ts - no issues

Notes and assumptions

• Only the incremental diff was reviewed. No other file changed in this increment, so earlier discussion threads on kilo-sessions.ts, rename-adoptions.ts, prompt.ts, and the Kilo session test suites were not re-litigated.
• Tests were not executed (read-only mode); CI-enforced checks (lint, typecheck, marker rules) were not run or commented on.

Previous Review Summaries (9 snapshots, latest commit 284d7e8)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 284d7e8)

Status: No Issues Found | Recommendation: Merge

Incremental review of c3f785bf..284d7e85 — a one-line test-only change that removes the tight 15_000 per-test timeout from refreshes effective config after project permission update so the suite uses the default timeout instead. This is the right direction for a load-sensitive server-route test: the assertions are unchanged, so the test still fails loudly on wrong behavior and no longer fails purely because a slow CI box exceeded 15s. No production code, no new resource/leak surface.

Files Reviewed (1 file in this increment)

• packages/opencode/test/kilocode/server/config-overlay.test.ts - no issues

Notes and assumptions

• Only the incremental diff was reviewed; existing discussion threads on kilo-sessions.ts, rename-adoptions.ts, prompt.ts, and the other test suites were not re-litigated and none of those files changed in this increment.
• The test.serial(name, fn,) call now spans multiple lines with no third argument; that is purely cosmetic and formatting is CI-enforced, so it is not flagged.
• Tests were not executed (read-only mode); CI-enforced checks were not run or commented on.

Previous review (commit c3f785b)

Status: No Issues Found | Recommendation: Merge

Incremental review of dccd6946..c3f785bf. Test-only increment on the queued-cancel regression test; both previous suggestions are addressed:

• Readiness now polls KiloSessionPromptQueue.snapshot(session.id).length >= 2. snapshot() returns only the waiting FIFO (the running slot's own message is removed when it takes over), so this genuinely proves both follow-ups are queued behind the in-flight turn before cancel runs — the weaker hasFollowup precondition is gone.
• Promise.allSettled no longer swallows every outcome: rejected results must stringify to /interrupt/i, so an unrelated provider/session failure of the in-flight prompt would fail the test instead of passing silently.

Files Reviewed (1 file in this increment)

• packages/opencode/test/kilocode/session-prompt-queue.test.ts - no issues

Notes and assumptions

• No production files changed in this increment; existing discussion threads on kilo-sessions.ts, rename-adoptions.ts, prompt.ts, and the other test suites were not re-litigated.
• Minor, not blocking: matching the rejection reason with /interrupt/i depends on how Effect stringifies an interrupted fiber failure from Effect.runPromise. If that ever changes, the assertion silently gets stricter rather than looser (the test fails loudly), which is the safe direction.
• No new caches, timers, subscriptions, or global patches — no new leak surface.
• Tests were not executed (read-only mode); CI-enforced checks were not run or commented on.

Previous review (commit dccd694)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	2

Incremental review of 3b0a65ff..dccd6946. The increment is test-only: the queued-cancel regression test swaps a 20ms Bun.sleep for a pollWithTimeout readiness wait and Promise.all for Promise.allSettled. Direction is right (published readiness signal over wall clock), but the new precondition is weaker than the test needs and the settle now hides all three outcomes.

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
packages/opencode/test/kilocode/session-prompt-queue.test.ts	785	hasFollowup is true once one follow-up is queued, so third can still enqueue after cancel bumps the version, run, and break the calls/assistants/user-message assertions. Poll snapshot(session.id).length >= 2 instead.
packages/opencode/test/kilocode/session-prompt-queue.test.ts	795	Promise.allSettled discards every rejection reason, so an unrelated failure of the in-flight prompt would keep the test green. Assert the tolerated reason (or document it).

Files Reviewed (1 file in this increment)

• packages/opencode/test/kilocode/session-prompt-queue.test.ts - 2 issues

Notes and assumptions

• No production files changed in this increment; existing discussion threads on kilo-sessions.ts, rename-adoptions.ts, prompt.ts, and the other test suites were not re-litigated.
• No new caches, timers, subscriptions, or global patches — no new leak surface.
• Tests were not executed (read-only mode); CI-enforced checks (lint, typecheck, tests, marker/changeset checks) were not run or commented on.

Fix these issues in Kilo Cloud

Previous review (commit 3b0a65f)

Status: No Issues Found | Recommendation: Merge

Overview

Incremental review of 14e9907d..3b0a65ff. The single commit is test-only: unseededMockSessionLayer now takes an explicit id and the three unseeded-title tests pass distinct ids (ses_unseeded_rename, ses_unseeded_adopt, ses_unseeded_auto), so the bootstrap share records written by each test no longer collide in process-global storage. get() still echoes the requested sid, so the mark/adoption assertions still target the session the handler sees. That resolves the only finding from the previous review; no production code changed in this increment.

Resolved since the previous review

• packages/opencode/test/kilocode/sessions/kilo-sessions-title.test.ts - the shared unseeded stub no longer hardcodes one session id, removing the cross-test coupling through the global session_share record.

Files Reviewed (1 file in this increment)

• packages/opencode/test/kilocode/sessions/kilo-sessions-title.test.ts

Notes and assumptions

• packages/opencode/src/kilo-sessions/kilo-sessions.ts and all other production files are unchanged in this increment; existing discussion threads there were not re-litigated.
• No new module-level caches, timers, subscriptions, or fetch patches - no new leak surface.
• Tests were not executed (read-only mode); CI-enforced checks (lint/prettier, typecheck, tests, marker and changeset checks) were not run or commented on.

Previous review (commit 14e9907)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	1

Overview

Incremental review of de91b698..14e9907d. The increment is test-only: the inline unseeded Session.Service stub is extracted into unseededMockSessionLayer(title) and two new tests cover the paths added by the previous commit — an unseeded session that consumes a rename mark (adopted, no POST) and one that consumes an auto-title mark (POST with generated: true).

Both tests are meaningful rather than vacuous: each pairs its POST assertion with a consume* check that fails if the handler never reached the mark branch, and neither would have passed before de91b698. That closes the previous review's coverage suggestion. The one remaining nit is that the extracted helper hardcodes a single session id, which couples the three unseeded tests through process-global share storage.

Issue Details (click to expand)

SUGGESTION

File	Line	Issue
packages/opencode/test/kilocode/sessions/kilo-sessions-title.test.ts	841	The extracted stub returns a fixed ses_unseeded id, so all three unseeded tests share the global session_share record written by bootstrap; a later test's readiness can resolve from an earlier test's record.

Resolved since the previous review

• The unseeded mark-consumption paths (kilo-sessions.ts:407-408) now have dedicated tests that distinguish the new behaviour from the pre-de91b698 early return.

Files Reviewed (1 file in this increment)

• packages/opencode/test/kilocode/sessions/kilo-sessions-title.test.ts - 1 issue

Notes and assumptions

• packages/opencode/src/kilo-sessions/kilo-sessions.ts is unchanged in this increment; the open design thread on the unseeded fall-through (generated: false on the first Updated of any kind) is left to its existing discussion and is not repeated here.
• The 200 ms holdTitlePosts window in the new adoption test doubles as the readiness wait for the handler; this is the same fixed-time synchronization already discussed on the existing thread at line 809, so it is not raised again.
• No new module-level caches, timers, or subscriptions in this increment — no new leak surface.
• CI-enforced checks (lint/prettier, typecheck, tests, marker and changeset checks) were not run or commented on.

Fix these issues in Kilo Cloud

Previous review (commit de91b69)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	1

Overview

Incremental review of f108af2a..de91b698. The increment is a single 3-line change: the prev === undefined early return is gone, so an unseeded session now runs consumeRenameAdoption / consumeAutoTitle before choosing an outcome.

That resolves the adoption-echo and mis-flagged-auto-title halves of the previous warning. What remains is the fall-through: with prev === undefined, sameTitle is always false, so the first Updated of any kind on an unseeded session (touch, setMetadata, setPermission, archive/share/summary/revert all publish Updated from patch) still POSTs the current local title as an explicit generated: false rename. Combined with last-write-wins explicit-rename semantics, a stale local title can overwrite a newer cloud-side one without any user rename. The behaviour added by this commit also has no test that distinguishes it from the previous code.

Issue Details (click to expand)

WARNING

File	Line	Issue
packages/opencode/src/kilo-sessions/kilo-sessions.ts	408	Residual from the previous review: with no matching mark and an unknown prev, a non-title Updated on an unseeded session (normal after restart — bootstrap seeding is capped at the default limit = 100) reports the local title as an authoritative rename and can clobber a newer cloud title.

SUGGESTION

File	Line	Issue
packages/opencode/src/kilo-sessions/kilo-sessions.ts	407	The two new unseeded paths (mark-based suppression, and generated: true from an auto-title mark) are untested; the existing unseeded test sets no marks and its generated: false assertion held before this commit too.

Files Reviewed (1 file in this increment)

• packages/opencode/src/kilo-sessions/kilo-sessions.ts - 2 issues

Notes and assumptions

• The generated: true / generated: false backend contract (auto-titles apply only over NULL/default titles, explicit renames are last-write-wins) is taken from the PR description; the clobber scenario above follows from that contract.
• Open threads on unchanged lines (the isPermanentHttpStatus 4xx classification at line 105 and the Effect.sleep(50) gate in the new title test) are left to their existing discussions and are not repeated here.
• No new memory-leak surface in this increment: no new module-level caches, timers, or subscriptions.
• CI-enforced checks (lint/prettier, typecheck, tests, kilocode_change markers, changeset presence) were not run or commented on.

Fix these issues in Kilo Cloud

Previous review (commit f108af2)

Status: 4 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	2
SUGGESTION	2

Overview

Incremental review of 172a8eab..f108af2a. Only one PR-authored commit is new (fix(kilo-sessions): report unseeded title updates); the rest of the range is a merge of main. The commit removes the seed outcome from the Session.Event.Updated title watcher so prev === undefined now reports the title with generated: false, plus one new test that fakes the unseeded state with a Layer.mock(Session.Service, …).

The one-line behaviour change is the risky part: it returns above consumeRenameAdoption / consumeAutoTitle, so an unseeded session bypasses the adoption suppression this PR added and claims an explicit rename for what may be an adopted inbound rename, an auto-title, or a title that did not change at all (Updated is published from patch, so touch / setMetadata / setPermission all reach the handler). Seeding covers only Created plus a project-scoped sessions.list() capped at 100 rows ordered by time_updated, so "unseeded" is the normal state for an older session resumed after a restart.

Issue Details (click to expand)

WARNING

File	Line	Issue
packages/opencode/src/kilo-sessions/kilo-sessions.ts	404	NEW: the prev === undefined early return skips both mark checks, so an unseeded session echoes adopted inbound renames back to ingest, reports auto-titles as generated: false (last-write-wins on the backend), and turns the first non-title Updated (touch, setMetadata, setPermission, archive/share/revert) into an authoritative rename that can clobber a newer cloud-side title.
packages/opencode/src/kilo-sessions/kilo-sessions.ts	105	Carried forward (open thread): isPermanentHttpStatus still marks every non-408/429 4xx permanent, so 401/403 (expired token, org membership not yet propagated) and 404 (title POST racing the server-side row) drop the rename for good, and no cache is cleared. Still disagrees with ingest-queue.ts retryable(), which treats 409/425 as retryable and routes 401/403 through onAuthError → clearCache().

SUGGESTION

File	Line	Issue
packages/opencode/test/kilocode/sessions/kilo-sessions-title.test.ts	839	NEW: reintroduces a fixed Effect.sleep(50) sync gate in a suite whose 50–300ms gates were replaced with pollWithTimeout earlier in this PR; test/AGENTS.md calls this an anti-pattern.
packages/opencode/src/kilo-sessions/kilo-sessions.ts	445	Carried forward (open thread): the else where an early return / single log.warn with a ternary message would do, plus isPermanent / isPermanentHttpStatus vs the single-word naming rule.

Files Reviewed (2 files in this increment)

• packages/opencode/src/kilo-sessions/kilo-sessions.ts - 1 new issue, 2 carried forward
• packages/opencode/test/kilocode/sessions/kilo-sessions-title.test.ts - 1 new issue

Notes and assumptions

• The generated: true / generated: false backend semantics (auto-titles apply only over NULL/default titles, explicit renames are last-write-wins) are taken from the PR description; the CLI-side consequence above follows from that contract.
• The merge of main (b6e913b5d2) brought ~95 unrelated files into the range; those were treated as upstream changes and not reviewed.
• The new test's whole-service Layer.mock is sanctioned by test/AGENTS.md, so it was not flagged, though a lighter real-service variant exists: create a real session (seeds via Created), emit Session.Event.Deleted to drop the knownTitles entry, then emit Updated.
• The new test remains non-vacuous for the branch it targets: it asserts { generated: false }, which the removed seed outcome would not produce.
• No new memory-leak surface in this increment (no new module-level caches or subscriptions); note the auto-title mark left unconsumed by the flagged branch still relies on the 60s prune.
• CI-enforced checks (lint/prettier, typecheck, tests, kilocode_change markers, changeset presence) were not re-run or commented on.

Fix these issues in Kilo Cloud

Previous review (commit 172a8ea)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	1

Overview

Incremental review of a3b5964a..172a8eab — one commit (refactor(kilo-sessions): simplify title reporting tests). kilo-sessions.ts moves restoreTitleState() from a separate if (!isPermanent) line into the existing else branch: behaviour-identical, and it removes the double test of the same boolean. kilo-sessions-title.test.ts extracts the copy-pasted ~18-line fetch stub into a shared titleTestFetch(requests, statuses, id) helper used by the 4xx / 5xx / 408-429 tests (-189/+150 net). Verified the helper is faithful to the stubs it replaced — the /title status lookup still keys on the local session id, which is what postSessionTitle puts in the URL (kilo-sessions.ts:1178), so the transient tests remain non-vacuous (a defaulted 200 would make the same-title re-POST assertions fail). No new issues found in this increment; the two entries below are earlier findings that remain live in changed code and already have open inline threads, so they were not re-posted.

Issue Details (click to expand)

WARNING

File	Line	Issue
packages/opencode/src/kilo-sessions/kilo-sessions.ts	105	isPermanentHttpStatus still marks every non-408/429 4xx permanent, so 401/403 (expired token, org membership not yet propagated) and 404 (title POST racing the server-side row) drop the rename for good: knownTitles keeps the title, consumed marks are not restored, and no cache is cleared, so authValid's cache keeps re-using the bad token. Still disagrees with ingest-queue.ts retryable(), which treats 409/425 as retryable and routes 401/403 through onAuthError → clearCache().

SUGGESTION

File	Line	Issue
packages/opencode/src/kilo-sessions/kilo-sessions.ts	445	Partially addressed by this commit (the redundant third branch is gone). Remaining: the else where an early return / single log.warn with a ternary message would do, plus isPermanent/isPermanentHttpStatus vs the single-word naming rule.

Resolved in this increment

• Test-stub duplication (kilo-sessions-title.test.ts) — the verbatim ~18-line fetch stub is now the shared titleTestFetch helper; only the gated-race test keeps a bespoke stub, which it needs for the promise gate.

Files Reviewed (2 files in this increment)

• packages/opencode/src/kilo-sessions/kilo-sessions.ts - 2 carried-forward issues
• packages/opencode/test/kilocode/sessions/kilo-sessions-title.test.ts - 0 issues

Notes and assumptions

• Confirmed the restoreTitleState() move is a pure reordering: it was previously called before the two log.warn calls and is now the first statement of the transient branch, with no intervening awaits or state reads.
• Two older findings remain only partially closed and already have active inline threads, so they were not re-posted: restoreTitleState still re-marks rename/auto-title adoption for a stale title even when the knownTitles guard declines to restore, and not_connected is still classified transient.
• No new memory-leak surface in this increment: no new module-level caches or subscriptions were added, and the per-test Maps are function-scoped.
• CI-enforced checks (lint/prettier, typecheck, tests, kilocode_change markers, changeset presence) were not re-run or commented on.

Fix these issues in Kilo Cloud

Previous review (commit a3b5964)

Status: 3 Issues Found | Recommendation: Address before merge

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	2

Overview

Incremental review of 843690ba..a3b5964a — one commit (fix(kilo-sessions): preserve newer title state) touching kilo-sessions.ts (+37/-7) and kilo-sessions-title.test.ts (+296). Both previous findings are addressed: restoreTitleState now only writes prev back when knownTitles still holds this handler's title (lost-update guard, covered by a new gated-fetch race test), and !ok no longer rolls back unconditionally — permanent failures keep the new title. Four new tests exercise the 4xx/5xx/408/429 and interleaving paths against the real watcher, which is the right level. The new failure classifier is where the remaining issues are.

Issue Details (click to expand)

WARNING

File	Line	Issue
packages/opencode/src/kilo-sessions/kilo-sessions.ts	105	isPermanentHttpStatus marks every non-408/429 4xx permanent, so 401/403 (expired token, org membership not yet propagated) and 404 (title POST racing the server-side row) drop the rename for good: knownTitles keeps the title, the consumed marks are not restored, and no cache is cleared, so authValid's 15-minute cache keeps re-using the bad token. It also disagrees with ingest-queue.ts:114-120 retryable(), which treats 409/425 as retryable and routes 401/403 through onAuthError → clearCache().

SUGGESTION

File	Line	Issue
packages/opencode/src/kilo-sessions/kilo-sessions.ts	446	Three branches on one boolean including an else; the two log.warn calls differ only in message and collapse to one call with a ternary. Also isPermanent/isPermanentHttpStatus vs the single-word naming rule.
packages/opencode/test/kilocode/sessions/kilo-sessions-title.test.ts	526	The ~18-line fetch stub is copy-pasted verbatim into all four new tests and re-implements mockFetch (:105) with a status override plus a local fetch that shadows the global; parametrizing the existing helper removes ~100 duplicated lines.

Files Reviewed (2 files in this increment)

• packages/opencode/src/kilo-sessions/kilo-sessions.ts - 2 issues
• `packages/opencode/test/kilocode/sessions/kilo

[Snapshot truncated.]

Additional previous summary content was truncated to keep this comment within platform limits.

---

_{Reviewed by claude-opus-5 · Input: 28 · Output: 3.7K · Cached: 589.9K}

_{Review guidance: REVIEW.md from base branch main}

[iscekic]

(bot) @kilocode-bot please review the current head after the origin/main merge. CI is green and all prior review threads are resolved.

[kilo-code-bot]

Code Review Summary

Status: 4 Issues Found | Recommendation: Address the WARNING before merge; suggestions optional

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	3

Overview

Review of head b6e913b5 — the merge of origin/main (38b7d576) into remote-cli-lifecycle-0b3a, plus a full re-review of the PR's net changes.

Merge verification: clean. git diff 172a8eab b6e913b5 (branch side) touches zero PR-owned files — the merge only fast-forwarded main's 95 files (i18n, jetbrains, skills inject, util/filesystem.ts, server test fixtures) into the branch. git diff 38b7d576 b6e913b5 is byte-identical in stat to the PR's net diff (13 files, +2402/−35), so the merge introduced no conflict resolution, no re-application, and no drift in kilo-sessions.ts / remote-sender.ts / rename-adoptions.ts / session/prompt.ts / the four test suites. No conflict markers anywhere under packages/opencode. Main's only opencode change after the merge base is src/session/llm.ts, which this PR does not touch — no new conflict surface. KiloSessions.Interface gained reportSessionTitle, and all four implementors in the tree (kilocode/tool/notify-user.ts, kilocode/tool/registry.ts, and the two test stubs) are updated, including the ones that arrived from main. All checks on head are green.

The net change reads correctly on the paths it targets: the strict create_session schema still rejects unknown fields (old-CLI degradation stays honest), orgId rides metadata so the claim is in the row before Created fires, meta() precedence (metadata → KILO_ORG_ID → auth) falls through instead of throwing on a get failure, the adoption/auto-title marks are consumed before the network hop with a guarded rollback, and the 5 new SessionPrompt cancel tests drive the real prompt/loop/cancel + KiloSessionPrompt.intake paths rather than re-implementing them. The one issue below is a coverage gap in the restart-seeding path that the new tests cannot see.

Issue Details (click to expand)

WARNING

File	Line	Issue
packages/opencode/src/kilo-sessions/kilo-sessions.ts	372-378	Bootstrap seeding of knownTitles calls sessions.list() with no limit, and listByProject defaults to limit = 100 ordered by time_updated desc (session/session.ts:1158). So after a CLI restart only the 100 most recently updated sessions in the project are seeded; for any older session the first Updated finds prev === undefined, classifies the change as {kind:"seed"}, and returns without POSTing — the rename is silently dropped exactly in the "first rename after process restart" case the seeding was added to fix (Decision 8). The new test (kilo-sessions-title.test.ts:466, "first rename after restart seeds from list and POSTs") uses a single session, so the 100-row cutoff is invisible to it. Mitigation is partial: heartbeat advertisements carry title (kilo-sessions.ts:692), so the backend may still converge — but the generation-aware POST (generated: false, last-write-wins) is the intended channel and it is skipped. Options: pass an explicit high limit, or drop the seed classification and treat an unknown prev as {kind:"report", generated:false} (idempotent last-write-wins; costs at most one redundant POST per session per restart).

SUGGESTION

File	Line	Issue
packages/opencode/src/session/prompt.ts	321-330	ensureTitle now reads fresh with orElseSucceed(() => null) and prepareAutoTitle returns false when fresh is null, so a transient Session.get failure makes the auto-title write skip entirely. ensureTitle only runs on the first real turn, so that session keeps its default title permanently — behaviour that the pre-change code (unconditional setTitle) did not have. Falling back to the already-validated input.session.title gate when the re-read fails keeps the new mid-generation protection without turning a read blip into a lost title.
packages/opencode/src/kilo-sessions/remote-sender.ts	77-80	SessionRenamedData.title is z.string().min(1) with no upper bound, so an adopted remote rename can write an arbitrarily long title into the session row and from there into every subsequent kilo_meta/session transport payload and heartbeat. Local auto-titles are capped at 100 chars (session/prompt.ts:319) and the sibling wire schema caps names explicitly (RemoteModelCatalog.MAX_NAME_LENGTH), so a .max(...) here would match the surrounding conventions.
packages/opencode/src/kilo-sessions/remote-sender.ts	53-57, 822-834	The new CreateSessionModel duplicates RemoteModelCatalog.ModelRef (providerID/modelID, both min(1)) and then diverges from it: the catalog the CLI advertises exposes ModelSelection as { model: {providerID, modelID}, variant } (variant a sibling), while create_session nests variant inside model, and send_message additionally accepts a bare "kilocode/…" string via normalizeModel that create_session does not. A client echoing currentModel from model_catalog must reshape it for create_session. Reusing RemoteModelCatalog.ModelRef.extend({ variant }) (or accepting ModelSelection) removes the duplicate schema and the shape mismatch. Related: neither agent nor model is validated against the agent list / provider catalog before Session.Service.create, so a bad value is ACKed at create time and only surfaces as a failure on the first prompt.

Previously tracked, still present in code (not re-filed)

Both entries from the last bot review remain live in kilo-sessions.ts at this head; their threads are resolved, so they are listed for the record only:

• isPermanentHttpStatus (:101-106) still classifies every non-408/429 4xx as permanent, so 401/403/404/409/425 drop the rename for good and no auth cache is cleared — still divergent from ingest-queue.ts retryable() (:114-121), which retries 409/425 and routes auth errors through clearCache().
• The else branch plus isPermanent/isPermanentHttpStatus naming (:445-460) vs the AGENTS.md early-return / single-word rules; the same applies to the new prepareAutoTitle / clearAutoTitleMark / restoreTitleState identifiers.

Files Reviewed (13 files, net diff 38b7d576..b6e913b5)

• packages/opencode/src/kilo-sessions/kilo-sessions.ts - 1 issue (+2 carried forward)
• packages/opencode/src/kilo-sessions/remote-sender.ts - 2 issues
• packages/opencode/src/kilo-sessions/rename-adoptions.ts - 0 issues
• packages/opencode/src/session/prompt.ts - 1 issue
• packages/opencode/src/kilocode/session/prompt.ts - 0 issues
• packages/opencode/test/session/prompt.test.ts - 0 issues
• packages/opencode/test/kilocode/sessions/kilo-sessions-title.test.ts - 0 issues
• packages/opencode/test/kilocode/sessions/remote-sender.test.ts - 0 issues
• packages/opencode/test/kilocode/sessions/rename-adoptions.test.ts - 0 issues
• packages/opencode/test/kilocode/sessions/ensure-title-mark.test.ts - 0 issues
• packages/opencode/test/kilocode/tool/notify-user.test.ts - 0 issues (interface stub only)
• packages/opencode/test/kilocode/tool-registry-indexing.test.ts - 0 issues (interface stub only)
• .changeset/remote-cli-lifecycle.md - 0 issues

Notes and assumptions

• No leftover merge artifacts: no <<<<<<</|||||||/>>>>>>> in packages/opencode, no duplicated handler bodies, and no broken imports — rename-adoptions stays a leaf module (imported by kilo-sessions.ts, remote-sender.ts, kilocode/session/prompt.ts, tests; imports nothing from them), and the shared-file additions in src/session/prompt.ts and test/session/prompt.test.ts are wrapped in kilocode_change markers.
• Verified the Updated handler's concurrency story holds: prev is read and knownTitles written in the same synchronous block after the awaited sessions.get, and both concurrent handlers re-read the row rather than trusting the event payload, so they converge on the current title instead of racing on stale payloads. restoreTitleState's ownership guard prevents the lost update.
• Verified Session.Metadata is Record<string, Any> and CreateInput.metadata is threaded to createNext, so the {orgId} claim persists in the row before Session.Event.Created — the create-order argument in the description checks out. resolveSessionOrg re-validates with Uuid.safeParse, so a schema-strictness mismatch degrades to fallback rather than a bad claim.
• The title POST reuses KILO_AGENT_NOTIFICATION_TIMEOUT_MS via agentNotificationTimeoutMs() for its readiness bound — functional, but the knob name now covers two unrelated features.
• Cancel tests: several assertions bound prompt.cancel with Effect.timeout("250 millis"/"1 second"). Correct as written, but these are the tightest budgets in prompt.test.ts and this suite already has load-sensitive tests on the Windows shards — worth watching for flakes rather than changing now.
• CI-enforced checks were not re-run; gh pr checks on this head reports every check passing.

[kilo-code-bot]

The review is already posted: #12704 (comment)

Summary: merge is clean, no drift in PR files. 1 WARNING (bootstrap sessions.list() seeding without a limit can miss renames on sessions outside the default 100 most-recent, defeating the seeding fix it's meant to provide) plus 3 suggestions (ensureTitle silently drops title on read failure, unbounded SessionRenamedData.title, CreateSessionModel/ModelRef schema duplication with no existence validation). Recommend addressing the WARNING before merge; suggestions optional.

[iscekic]

(bot) Kilobot posted no approving summary on this head after two retriggers

[iscekic]

(bot) @kilocode-bot please review current head f108af2. Fixed the restart title-seeding WARNING with focused coverage; prior threads remain resolved.

[kilo-code-bot]

Code Review Summary

Status: 0 Issues Found (this increment) | Recommendation: Approve — WARNING resolved with a verified fix and non-vacuous test.

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	0 (this increment)

Overview

Review of head f108af2ad0 — the single commit fix(kilo-sessions): report unseeded title updates, diffed against the last-reviewed head b6e913b5 (from this comment).

Diff is exactly the fix the WARNING asked for. The entire change is 2 files, +75/−2:

packages/opencode/src/kilo-sessions/kilo-sessions.ts                          | 3 +-
packages/opencode/test/kilocode/sessions/kilo-sessions-title.test.ts          | 74 +++++++++

In kilo-sessions.ts, the outcome classifier in the Session.Event.Updated handler drops the { kind: "seed" } branch and its type entirely:

- if (prev === undefined) return { kind: "seed" }
+ if (prev === undefined) return { kind: "report", generated: false }

This is Option 2 from the prior WARNING verbatim ("drop the seed classification and treat an unknown prev as {kind:"report", generated:false}"). Previously, any session whose title wasn't in the bootstrap knownTitles map — i.e. any session outside the default 100-row sessions.list() page after a restart — silently dropped its first post-restart rename because prev === undefined matched "seed" and the handler returned before POSTing. Now the same prev === undefined case is treated as an ordinary, idempotent, last-write-wins report (generated: false), so the rename reaches the ingest title route regardless of whether the bootstrap list happened to include the row.

Verification performed (not just reading the diff):

1. Ran the new test title report: unseeded session (sessions.list() misses it, prev undefined) reports rename with generated:false at f108af2ad0 — passes, and asserts the exact POST body { title: "Renamed Title", generated: false }.
2. Reverted only the 3-line kilo-sessions.ts hunk (kept the new test) and re-ran the same test — it fails with error: title POST never fired for unseeded session, confirming the test is not vacuous and genuinely exercises the pagination-gap path the WARNING described (Session.Service.list mocked to return [], get returns a session never seen by Created/bootstrap, so prev is truly undefined).
3. Restored the fix and ran the full title/adoption/mark suite (kilo-sessions-title.test.ts, rename-adoptions.test.ts, ensure-title-mark.test.ts): 23 pass, 1 pre-existing failure (meta org precedence: session metadata > KILO_ORG_ID > auth accountId) — reproduced identically at b6e913b5 (before this fix) in the same sandbox, so it's an environment/auth-lookup artifact unrelated to this change, not a regression. gh pr checks on this head shows every CI job green, including unit (linux/macos/windows), so this doesn't reproduce in CI.
4. git log --oneline b6e913b5..f108af2ad0 shows exactly one commit, no merge; git diff has no <<<<<<</=======/>>>>>>> artifacts. Clean, minimal, single-purpose commit.

Note (non-blocking): the doc comment directly above knownTitles (kilo-sessions.ts:368-371) still says bootstrap seeding exists "so the first real rename … is not treated as a seed-only sighting and dropped" — that phrasing is now slightly stale since the seed classification it refers to no longer exists; the comment describes the old mechanism the WARNING flagged, not the new fallback-report mechanism that actually closes the gap today. Cosmetic only, no behavior impact — not filing as a SUGGESTION since it doesn't affect correctness, just flagging for awareness. The underlying sessions.list() call at bootstrap is still unbounded/un-paginated, but that's no longer load-bearing for correctness given the new fallback: even a session the bootstrap list never saw now gets its title reported on its first real Updated.

Prior SUGGESTIONS from `b6e913b5` — status check (no action requested, per the task)

None of these were touched by this commit; all three remain exactly as before:

• SessionRenamedData.title unbounded (remote-sender.ts:79) — still z.string().min(1) with no .max(...). An adopted remote rename can still write an arbitrarily long title into the session row/transport payloads. Unchanged.
• ensureTitle silently drops title on read failure (session/prompt.ts:319-330) — fresh is still sessions.get(...).pipe(Effect.orElseSucceed(() => null)), and prepareAutoTitle still returns false (skipping the write entirely) when fresh is null, with no fallback to the already-validated input.session.title gate. Unchanged.
• CreateSessionModel / RemoteModelCatalog.ModelRef schema duplication (remote-sender.ts:53-57 vs remote-model-catalog.ts:19-33) — still two independently-defined {providerID, modelID} schemas with the variant nesting mismatch (ModelSelection has variant as a sibling of model; CreateSessionModel nests it inside). Unchanged.

Previously tracked, still present in code (carried forward, not re-filed)

Unaffected by this commit, threads resolved:

• isPermanentHttpStatus (kilo-sessions.ts) still classifies every non-408/429 4xx as permanent (divergent from ingest-queue.ts retryable()).
• The else branch plus isPermanent/isPermanentHttpStatus/prepareAutoTitle/clearAutoTitleMark/restoreTitleState naming vs. AGENTS.md's early-return / single-word conventions.

Files Reviewed (2 files changed since b6e913b5)

• packages/opencode/src/kilo-sessions/kilo-sessions.ts — 0 issues (WARNING resolved)
• packages/opencode/test/kilocode/sessions/kilo-sessions-title.test.ts — 0 issues (new test verified non-vacuous by reverting the fix and re-running)

Notes and assumptions

• Merge/branch cleanliness: single non-merge commit on top of b6e913b5; no conflict markers; no unrelated files touched.
• CI on this head: all checks passing per gh pr checks 12704 (one Windows shard was still pending at review time; all others, including Kilo Code Review, unit (linux), unit (macos), unit (windows) shards already reported, typecheck*, and the annotation/kilocode_change/markdown guards, are green).
• The local meta org precedence test failure is reproducible at both b6e913b5 and f108af2ad0 in this sandbox and does not reproduce in CI — treated as a pre-existing environment artifact, not attributed to this increment.
• This review only covers the increment since the last-reviewed head (b6e913b5 → f108af2ad0), per the task; it does not re-review the full PR net diff.

[kilo-code-bot]

The review's already posted and it's a clean approve — WARNING resolved, verified fix, no new issues. Nothing further needed from me here.

[iscekic]

(bot) @kilocode-bot please review current head de91b69 after resolving the unseeded mark handling warning.

[kilo-code-bot]

Code Review Summary

Status: 1 Issue Found (this increment) | Recommendation: Approve — the WARNING is genuinely resolved; the only finding is a missing regression test for the fixed path.

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	1

Overview

Review of head de91b69898 diffed against the last-reviewed head f108af2ad0 (prior review).

The increment is one commit, one file, +3/−1 (fix(kilo-sessions): consume unseeded title marks):

packages/opencode/src/kilo-sessions/kilo-sessions.ts | 4 +++-

   if (sameTitle) return { kind: "same" }
-  if (prev === undefined) return { kind: "report", generated: false }
+  // Consume marks before the network hop so the 60s TTL does not span
+  // token resolution + ingest.sync. Checks run even when prev is
+  // unknown — an unseeded mark must not leak past this handler.
   if (consumeRenameAdoption(sessionID, session.title)) return { kind: "adopted" }
   return { kind: "report", generated: consumeAutoTitle(sessionID, session.title) }

2. Is the "unseeded mark handling" WARNING resolved? Yes.

The original WARNING (thread on kilo-sessions.ts) had two halves. f108af2ad0 fixed the drop half (unseeded titles are reported instead of silently seeded), but the replacement line still returned above consumeRenameAdoption / consumeAutoTitle, so on an unseeded session both mark checks were still skipped. This commit removes that early return, which closes each consequence the WARNING named:

• Inbound rename echoed back as a local rename. session.renamed → markRenameAdopted → setTitle → Updated. With prev === undefined the adoption is now consumed, the classifier returns {kind:"adopted"}, and the handler returns before reportSessionTitle — no echo to the ingest title route. restoreTitleState() correctly re-marks the adoption and knownTitles.delete(sessionID) (rather than set(prev)) on an ingest failure, so the unseeded rollback shape is right too.
• Auto-title mis-flagged as an explicit rename. An ensureTitle auto-title on an unseeded session now POSTs generated: true, so the backend applies it only over NULL/default titles instead of clobbering a real user/cloud rename under last-write-wins.
• Lingering mark swallowing a later real rename. The mark is consumed in-handler instead of surviving until the 60s MARK_TTL_MS prune, where it could have suppressed a genuine subsequent local rename to the same string.

The change is non-vacuous: it flips observable behavior on the unseeded path (suppressed POST for adoptions, generated: true instead of false for auto-titles).

3. New issues introduced? None found.

Checked the interactions the earlier rounds churned on, all still hold:

• Concurrency. knownTitles is still advanced optimistically before await ingest.sync, so a second concurrent Updated on the now-seeded session sees sameTitle and cannot double-POST with a wrong flag. The only window this commit widens is "both marks are checked on the first sighting", which is strictly one map lookup — no new await, no new ordering.
• Rollback ownership. restoreTitleState() still guards on knownTitles.get(sessionID) === session.title before restoring, and the prev === undefined branch was already present, so it was written to handle exactly this case. Re-marking is consistent with the new outcome values (adopted → markRenameAdopted, report+generated → markAutoTitle).
• Same-title path. if (outcome.kind === "same") consumeRenameAdoption(...) after sync is untouched and still non-redundant (the same branch never reaches the classifier's consumes).
• Bounded-ness. Consuming a mark cannot create extra traffic: worst case an unseeded non-title Updated (touch/setArchived/etc.) consumes a matching auto-title mark and POSTs once with generated: true, then the real title Updated is a sameTitle no-op. Net one POST per session per process, with a more accurate flag than before this commit.
• Typecheck/annotation/table/source-link guards and unit (macos) are green on this head; linux/windows shards were still running at review time. git log f108af2ad0..de91b69898 is a single non-merge commit, no conflict markers, no unrelated files.

4. Test coverage for the fix — this is the one gap

SUGGESTION: the behavior change in this commit has no locking test; reverting the one-line deletion leaves the suite green.

The existing unseeded test (kilo-sessions-title.test.ts:780, "title report: unseeded session (sessions.list() misses it, prev undefined) reports rename with generated:false") never sets a mark, so with no renames/autos entry both consumes return false and the outcome is {kind:"report", generated:false} either way — it asserts the f108af2ad0 fix, not this one. Conversely, every test that does set a mark (:366 markAutoTitle, :390/:420/:425 markRenameAdopted) creates the session through the real Session.Service, so Session.Event.Created seeds knownTitles and prev is defined — none of them enters the branch this commit changed.

So the fixed path is exercised by neither side of the suite. Two cheap cases on the existing mockSessionLayer fixture (or the real layer plus clearAll() + a knownTitles-free session) would lock it:

1. unseeded + markRenameAdopted(id, "Cloud title") → no /title POST, and consumeRenameAdoption(id, "Cloud title") === false afterwards (proves the mark was consumed, not merely bypassed);
2. unseeded + markAutoTitle(id, "Auto title") → exactly one POST with { title: "Auto title", generated: true }.

Without (1) in particular, a future refactor can silently restore the echo-a-cloud-rename-back-as-an-explicit-rename bug that this WARNING was filed for. Not blocking — the production change is small, reviewed line by line, and clearly correct — but it is the one thing that keeps this increment below the verification bar the rest of this PR has met (every earlier fix in this PR shipped with a test that fails when the fix is reverted).

Cosmetic notes (not filed as issues)

• The new comment repeats, verbatim, the first two lines of the block comment ~12 lines above (kilo-sessions.ts:385-389 already says "Consume marks before the network hop so the 60s TTL does not span token resolution + ingest.sync"). Only the third sentence ("Checks run even when prev is unknown…") is new information; trimming the duplicate would keep the handler comment set readable.
• The knownTitles doc comment (:368-371) still explains bootstrap seeding in terms of "not treated as a seed-only sighting and dropped". The seed classification it describes was deleted in f108af2ad0; flagged in the previous round, still stale.

Previously filed items — status

Unchanged by this commit, threads already resolved/triaged; not re-filed: unbounded SessionRenamedData.title, ensureTitle dropping the title on a read failure, CreateSessionModel / ModelRef schema duplication, isPermanentHttpStatus diverging from ingest-queue.ts retryable() (author-triaged as intentional), and the else/isPermanent* naming vs. AGENTS.md conventions.

Verification method and limits

• Verified statically: read the full Session.Event.Updated handler and rename-adoptions.ts at de91b69898, and traced every mark producer/consumer plus each mark-setting test's seeding path to establish the coverage gap above.
• This sandbox has no installed dependencies (node_modules absent), so I did not re-run bun test ./test/kilocode/sessions/kilo-sessions-title.test.ts here, and I did not do the revert-and-rerun check used in the previous round. The coverage conclusion follows from the fixture inspection, not from a run; unit (macos) is green on this head, which is consistent with the change being test-neutral.
• Scope: increment f108af2ad0 → de91b69898 only; the full PR net diff is not re-reviewed.

[kilo-code-bot]

Review already posted: #12704 (comment) — Approve, 0 CRITICAL/WARNING, 1 SUGGESTION (add a locking test for the unseeded-mark branch, since existing tests don't exercise it either way). No further action needed unless you want the suggested test added.

[iscekic]

(bot) @kilocode-bot please review current head 14e9907; unseeded mark coverage added and all current threads are resolved.

[kilo-code-bot]

@/workspace/9d278969-5453-4ae3-a51f-a8d2274a7b56/ce12ef3d-ae95-4d77-b4f0-23735f0a0591/sessions/agent_d3c627f5-8cdf-43ed-ab97-83ab22cbb40e/.review-12704.md

[kilo-code-bot]

Review already posted for head 14e9907d21: #12704 (comment)

0 CRITICAL / 0 WARNING / 1 SUGGESTION — Approve. No production diff in this increment; the added test coverage is genuine and non-vacuous. Only remaining note is a non-blocking suggestion about the untested rollback path on ingest.sync failure. Nothing further needed from me unless you want that rollback case covered too.