Repository navigation
fix(mobile): blank the approval count when a glanceable frame lapses - #8573
Conversation
A frame that lapses in place cleared every work count except the approval count, so a stale surface kept a count for an ask the publisher had already retired. The frames built from no rows already blanked it. Both producers now spread one blank-work record from the shared snapshot module, so a future work field cannot be cleared by one path and survive the other. Tests drive the fetch-error path past the deadline, the orphaned-org republish, and the Live Activity content state.
iscekic
added
drain
|
bot: Runtime proof for the expired frame, captured from the Android dev client against the local dev stack on a work VM. Exercised: an owned fixture account with one tray row whose agent waits on a permission prompt. The app published a happy frame carrying the approval count, the backend was then made unreachable while the app stayed online, the frame passed its deadline, and the app's query reported an error on the next refresh. Observed on the published frames:
The shipped deadline is eight hours, so the run lowered that constant locally to make the deadline reachable in one sitting; both the constant and the diagnostic log line were reverted before the commit, which is why this comment carries the observation rather than the branch. No screenshot: nothing visible changes. Every control was already gated on the wait count, so an expired card drew no Approve before or after this change. The fix removes the inconsistent count from the frame itself, which no surface rendered distinctly. |
![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 only change since the prior review is Assumptions: Files Reviewed (1 file)
Fix these issues in Kilo Cloud Previous Review Summary (commit aef69bb)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit aef69bb)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe expiry fix itself is correct and well covered; the only finding is that the new shared blank-work record does not actually enforce the future-field guarantee its comment advertises. Assumptions: Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (8 files)
Reviewed by deepseek-flash-latest · Input: 28.2K · Output: 3.4K · Cached: 197.8K Review guidance: REVIEW.md from base branch |
…field The record's field list was hand-written, so a work field added to the snapshot schema would compile while both producers kept omitting it. The annotation now covers every field the frame itself does not describe, so a new work field is a type error here until it is blanked.
iscekic
added
the
merge:bot
What changed
An expired glanceable frame now carries no work at all. The lapsed-in-place frame kept its approval count while every other work count, the wait anchor, and the scheduled wake were already blank, so the surface could hold a count for an ask the publisher had retired in the same transition.
packages/app-shared/src/glanceable-agents-snapshot.ts: one exported blank-work record (running,needsInput,needsApproval,idle,scheduled,needsInputSince,scheduledAt) typed against the snapshot, so a later work field cannot be cleared by one producer and survive the other.apps/mobile/src/lib/glanceable/snapshot-transforms.ts: the expiry branch ofwithStatusspreads that record instead of a partial literal.withStatusstill does not renewexpiresAt, and the newest-result fact is untouched.apps/mobile/src/lib/glanceable/cleanup.ts: the terminal snapshot spreads the same record for the same fields.Tests
publisher.test.tsdriveshandleFetchErrorthrough a real sink past the deadline with a permission row in the tray: the stale frames keep the approval count, both expired frames blank it.cleanup.test.tsseeds an approval count and asserts the orphaned-org republish keeps it while stale and clears it on expiry.view-props.test.tsbuilds the Live Activity content state from the expired frame and asserts no work count remains, the approval count included.active-agents-live-activity.test.tscovers the approve gate on a frame with neither count.All three new assertions fail on the previous transform with
needsApproval: 1and pass with this change.Verification
pnpm exec vitest run src/lib/glanceable src/glanceable-ios src/glanceable-androidinapps/mobile: 61 files, 1132 tests pass.apps/mobilevitest for the touched files on the pre-fix transform: 3 failures, eachneedsApproval: 1where the new assertion expects 0.pnpm --filter @kilocode/app-shared run typecheck: clean.apps/mobiletypecheck reports the same error count asmainin this checkout (workspace packages unbuilt), with no new error in a touched file.Runtime proof on the Android emulator
Local dev stack plus the Android dev client on this VM, signed into an owned fixture account, with a real tray row whose agent waits on a permission prompt. The app published a happy frame carrying the approval count, the backend was made unreachable while the app stayed online, the frame passed its deadline, and the app's query then reported an error. The published frame:
The expiry deadline is 8 hours in the shipped code; the run lowered that constant locally so the deadline was reachable in one sitting. The log line and the constant were reverted before this commit.
No screenshot: nothing visible changes. Every control was already gated on the wait count, so the expired card drew no Approve before or after; the fix removes the inconsistent count from the frame itself, which no surface rendered distinctly.
Proxy answers
The owner's proxy gave these answers in the owner's place.
Answer: Shared blank-work record in app-shared
Confidence: 0.75 · Precedent: Fix the root cause and the class of the issue: session 00eb2a1d3e4b, "if anything repeats the same mistake over and over again, fix the class"; also extract shared code into an internal library: session 5d8113f51c48
Answer: Keep it, blank only the work counts
Confidence: 0.50 · Precedent: Put each separate concern in its own PR: session debabfb38c82, owner asked for the change "in a separate PR"
Answer: Keep the production-path tests: handleFetchError past the deadline through a real sink, the cleanup republish, and the iOS Live Activity content state. Run only those suites plus a narrow typecheck. Also capture one real runtime proof on the Android emulator on the VM, against the dev stack with a real session that has a pending approval. Cut the network or the backend so that handleFetchError runs past the deadline, then log the published expired frame and show that needsApproval is 0 and the other wait fields are cleared. Post those logs in a PR comment. No screenshot is needed because nothing visual changes; say so in the PR. Add missing-e2e only if the emulator run truly cannot reach the expiry path.
Confidence: 0.45 · Precedent: Proof and verification: require valid e2e proof as logs or screenshots that make sense, and use real backend state when a fixture cannot mimic real usage (session 55c9c80332a7: "logs and/or screenshots are mandatory proof"; session b1be07191a1d fixture rules)