Repository navigation
fix(mobile): bind needs-input acks to the answered raise - #8583
Conversation
An ack stored only a raise-less marker, and the reconciler then adopted whichever attention raise it observed next, so a question or permission raised after an answer — while the app was backgrounded, offline, or closed — kept its badge hidden, including after a restart. Acks now name the raise the user answered (the row's status time when it carried one, the status otherwise), reconcile resolves an ack only for its own raise, and a hydrated ack that names no raise suppresses nothing. Raise ids compare as instants through the shared Hermes-safe timestamp reader, because stored rows hand over PostgreSQL text and live rows ISO 8601. Readers that only have a status still match the acked status. The answer, skip, permission, notification and glanceable approve paths all pass the raise they answered.
iscekic
added
drain
|
bot: End-to-end run on the Android emulator against the local dev stack, signed in as a seeded Observed, in order:
The badge-pixel area of the tab bar is red-dominant in captures 2, 4 and 5 and not in capture 3, which
|
The new ack contracts pushed the interaction-handlers suite past the max-lines budget, and the badge hydration case nested two ternaries in one expectation. Split the count into a named local and declare the suite's single-file budget the way the neighbouring suites do.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe incremental revision adds relay-side persistence for authoritative status notifications and collision handling for replaced CLI sockets; reading the changed lines and their callers found no new correctness, security, or leak issues. Verification is by code inspection; the relay test suite was not executed in this read-only session ( Files Reviewed (2 files)
Previous Review Summaries (4 snapshots, latest commit 815510d)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 815510d)Status: No Issues Found | Recommendation: Merge Executive SummaryThis incremental revision closes the carried-forward finding: enrichment now emits Assumptions: Files Reviewed (10 files)
Previous review (commit f89c948)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental revision adds a mounted tab-badge test for the same-status re-raise, which is sound; the unresolved risk remains that a tray row can pair a live attention Assumptions: Overview
Issue Details (click to expand)WARNING
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Previous review (commit 6d8b1ff)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe two prior ack-identity findings are addressed in this revision, but a tray row can still pair a freshly observed attention Assumptions: Overview
Issue Details (click to expand)WARNING
Files Reviewed (13 files)
Previous review (commit 5b28440)Status: 3 Issues Found | Recommendation: Address before merge Executive SummaryThe new raise-bound ack is not honoured by the status-only observers, so a same-status re-raise can stay hidden and a tray row whose Assumptions: Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (22 files)
Reviewed by deepseek-flash-latest · Input: 101.2K · Output: 31.8K · Cached: 2.2M Review guidance: REVIEW.md from base branch |
The tray badge, launcher surfaces and Agents rows read a row they already have the status time for, so reading the ack by status alone kept a same-status re-raise hidden. `isAttentionAcked` now takes the raise a reader sees (identity plus status): a row that cannot be dated, or an ack written from a status-only answer, still matches by status, while a dated row is matched against exactly the raise it names. An ack no longer reuses a raise the store has already answered; only a cleared entry tracks the raise still waiting for an answer.
Mounts the real badge with tray rows that carry their status time, acks the dated raise, then pushes a later permission under the same status and expects the count back.
|
bot: Re-verified on the emulator after the reviewer-driven change to how readers match a raise (the Ran the same flow again on the real app against the local stack with the published CLI as the remote
Row crops read
|
iscekic
added
merge:blocked:kilobot-not-ready
iscekic
removed
merge:blocked:kilobot-not-ready
|
bot: Verified on the real Android development app with an owned fake account, an isolated local backend/database, and the published CLI 7.8.15 running as the remote. Optional analytics was disabled; inference used the deterministic local fake model. Observed on the final implementation:
This run initially reproduced a real failure: a refresh could replace a newer idle state with an already-answered question from an older relay snapshot. The final checks above were performed after the relay snapshot fix, without temporary source instrumentation. Limitations: permission responses and forced delayed-answer ordering were not exercised on device. A pending question's answer card did not reliably restore after leaving/reopening its detail during this run, so the final answers were submitted while the real detail remained mounted; pending-interaction replay is not claimed as verified here. Attachments are focused composites of the session-row and Agents-tab regions from real captures, omitting unrelated navigation. They show only owned fake-session data: attention before answering, the cleared state after restart, and a new attention request after restart.
|
Evidence
Evidence: demonstrated — failing-before regressions reproduced submission races, and the real Android app with the published CLI reproduced post-answer badge resurrection; the relay fix kept the badge cleared through refetches and heartbeats while later unanswered raises, including after restart, remained visible.
Change
statusUpdatedAt ?? status), including across app restarts. A later dated raise keeps its badge; legacy persisted acknowledgements without an identity are discarded.statusUpdatedAtonly when its stored status matches the resolved status. Mobile heartbeat and snapshot merges preserve the timestamp only while its status remains effective. An explicit undated status transition drops the prior status's timestamp.Verification
Follow-up
pnpm --filter kilo-app testcompleted successfully.git diff --checkpassed.UserConnectionDOsuite passes 312 tests, including attachment persistence, hibernation, owner transfer, same-ID reconnect, cold disconnect cleanup, null status, and independent child attention.pnpm --filter cloudflare-session-ingest exec vitest run --maxWorkers=2 --testTimeout=15000— 37 suites, 1,400 tests passed, 3 skipped. The unchanged oversized-parser test exceeded the default 5-second timeout on this loaded VM; the reported full run uses explicit headroom rather than changing that test.Follow-up Android and CLI proof
Ran the real Android development app against an isolated local backend/database using an owned fake account and the published CLI 7.8.15, with deterministic local fake-model inference and optional analytics disabled.
Device limits: permission responses and forced reversed submission completion are covered by regressions, not device proof. The pending question card did not reliably restore after leaving/reopening detail during this run; final answers were submitted through the real app while detail stayed mounted. Pending-interaction replay is not claimed as verified. Heartbeats have no producer transition identity/time, so subsequent undated heartbeats remain admissible under the existing contract.
Initial-session device evidence
The initial session exercised the real Android app against the local stack with an owned fake account, the published CLI as the remote agent, and the deterministic fake model. It observed NEEDS INPUT and a tab badge for a question, badge clearance after answering, and the badge returning for a later unanswered question. After answering and force-stopping/relaunching the app, a third question showed the badge again. Screenshots are attached in the existing PR comments. Those runs did not force out-of-order network completion; that ordering is covered by the regressions above.
Proxy answers
The owner's proxy gave these answers in the owner's place.