[gjkim42]

What type of PR is this?

/kind feature

What this PR does / why we need it:

Reading a Session result in one browser leaves its unread marker visible in other browsers. Store the latest acknowledged completion with the Session so read state is shared across browsers and devices for the same signed-in user. Token-authenticated browsers share one read state.

The Console acknowledges a result only when its ending is visible in the focused Conversation. Other browsers receive the acknowledgment on their next Session refresh, normally within five seconds. Temporary save failures keep the marker unread and retry on refresh. Rejected requests (400, 403, and 404) stop retrying for that result until the page is reloaded; polling and visibility events do not restart them. A result seen before its status is published remains pending even if the user leaves that conversation.

Before — the second browser still shows the result as unread after it was read in the first browser

After — the same second browser clears the marker and count on refresh

Which issue(s) this PR is related to:

N/A

Special notes for your reviewer:

• Acknowledgments are stored in Session annotations, one value per authenticated username. They survive Console restarts and are removed with the Session. Each write prunes receipts for superseded results while preserving current readers and unrelated annotations. No CRD changes or new storage service are required.
• The endpoint checks get sessions and patch sessions, verifies the Session UID and current completion ID, and uses optimistic locking. Stale acknowledgments cannot clear newer results or results from a replaced Session.
• Removed browser-only receipt storage, pruning, storage-event synchronization, and their tests. The tests and reference now describe shared read state.
• Tests cover permanent rejection suppression, transient retries, receipt pruning, user isolation, and concurrent updates. Invalid annotation writes return 400 so they are not retried as server outages.
• Validation: make update, make verify, and make test passed. Unit tests ran with agent-specific CODEX_HOME and CODEX_AUTH_JSON variables unset.
• make test-integration TEST_FLAGS='-run TestConsole' passed: Console membership, OIDC, and GitHub authentication tests. The proxy tests use separate authenticated cookie jars to verify shared acknowledgments against Kubernetes.
• Chromium checked the actual frontend with API and WebSocket fixtures in independent browser contexts, reproduced the baseline behavior, verified synchronization and reload persistence, and captured the matching screenshots above. Screenshots are PR attachments, not repository files.

Does this PR introduce a user-facing change?

Session unread markers synchronize across browsers and devices for the same signed-in user. Reading a result clears its marker in other browsers on their next refresh. Token-authenticated browsers share a common read state.

---

Summary by cubic

Reading a Session result in one browser no longer leaves its unread marker visible in other browsers. Read acknowledgments are now stored with the Session, so browsers and devices for the same signed-in user share the read state; token-authenticated browsers share one state.

Key changes

• The frontend acknowledges a result only when its ending is visible in the focused conversation, and other browsers pick it up on the next Session refresh, normally within five seconds.
• A new POST /api/sessions/{namespace}/{name}/read endpoint stores the acknowledgment as a Session annotation using optimistic locking, and now requires get sessions and patch sessions permissions; stale acknowledgments cannot clear newer results or replaced Sessions.
• Failed saves keep the marker unread and retry on refresh; the previous browser-local receipt storage, pruning, and storage-event synchronization were removed along with their tests.

^{Written for commit baae288. Summary will update on new commits.}

[gjkim42]

/kelos review

[kelos-bot]

🤖 Kelos Task Status

Task kelos-reviewer-issue-comment-3d341f11e50d has succeeded. ✅

[cubic-dev-ai]

All reported issues were addressed across 13 files

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[kelos-bot]

🤖 Kelos Reviewer Agent @gjkim42

Review Summary

Verdict: APPROVE
Overall correctness: patch is correct
Scope: Moves Session unread-completion receipts out of browser localStorage and into per-user Session annotations. A new POST /api/sessions/{ns}/{name}/read endpoint writes them, and readNotificationId is now returned on every Session summary.

Findings Overview

Priority	Count	File:Line	Summary
P0	0	—	none
P1	0	—	none
P2	1	internal/consoleserver/frontend/app.ts:1950	Acknowledgments that can never succeed (403) are re-sent on every poll and scroll frame
P3	0	—	none

Findings

Correctness

• [P2] internal/consoleserver/frontend/app.ts:1950 (mirrored in web/app.js): acknowledgments that can never succeed are retried forever.
  • What happens: saveCompletionRead swallows every error and keeps the entry in pendingCompletionReads. syncCompletionReads (app.ts:1927) then re-POSTs it on every 5 s loadSessions poll. While the conversation is open, markVisibleCompletionRead also re-POSTs it on every scroll, focus, or renderSessions frame.
  • Who hits it: users whose RoleBinding grants get/list sessions but not patch sessions, i.e. a custom read-only role. The built-in kelos-console-user role includes patch, so default users are not affected.
  • Effect: for these users the request always returns 403 and the marker never clears. On main their browser-local receipts worked. Each Session they view with a result adds another pending entry, and that entry is retried until the result changes. In OIDC/GitHub mode every retry costs two SubjectAccessReviews and two Console access reviewed log lines, so this becomes steady background load per open browser.
  • Suggested fix: separate failures that will not succeed on retry (403, plus 400/404) from transient ones (409 optimistic-lock conflict, 5xx, network errors). For example, have api() attach response.status to the thrown error, and drop the pending entry on permanent statuses, optionally keeping a tab-local read so the marker still clears.
  • Test: add a case to unread_completions_test.js checking that a 403 is not re-sent on the next syncCompletionReads.

Key takeaways

• The server side holds up. The handler checks the Session UID, the current lastNotification ID and type, and that no reset is in progress, then writes with MergeFromWithOptimisticLock. A stale or concurrent acknowledgment therefore can't clear a newer result or a replaced Session.
• The generated annotation key is always a valid qualified name. The last base64url character of a 32-byte digest carries only 4 bits, so it can never be - or _. Notification IDs are scoped by journal UUID, so the receipts stay correct across resets and clones.
• The only gap is the client's retry policy for permanent failures, described above.

[gjkim42]

Addressed the latest reviews in 92e6926 and kept the PR as one commit.

• Kelos review: API errors retain their HTTP status. Results rejected with 400, 403, or 404 stop retrying until page reload, including retries triggered by polling, scrolling, and focus. The marker stays unread until a server acknowledgment is received. Network errors, conflicts, rate limiting, and server outages remain retryable.
• Cubic: receipt writes prune entries for superseded results, preserving current readers and unrelated annotations. Invalid annotation writes return 400.
• Tests cover rejected-result suppression, transient retries, acknowledgment from another browser after a local rejection, receipt pruning, and error classification. Updated the reference and PR notes.

Passed make update, make test (agent-specific CODEX_HOME and CODEX_AUTH_JSON unset), make test-integration TEST_FLAGS='-run TestConsole', and make verify. CI is rerunning on the pushed commit.

[cubic-dev-ai]

1 issue found across 7 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/consoleserver/completion_reads.go">

<violation number="1" location="internal/consoleserver/completion_reads.go:66">
P2: This cleanup deletes unknown or newer receipts, not just known superseded ones. Preserve receipt values that cannot be proven older than `notification.ID`; otherwise a stale status/read can erase another browser’s acknowledgment and make that result unread again.

(Based on your team's feedback about preserving unknown newer completion receipts.) [c0c5fc9e-ea64-47bc-9858-055049e42411]</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic</sub>

[cubic-dev-ai]

P2: This cleanup deletes unknown or newer receipts, not just known superseded ones. Preserve receipt values that cannot be proven older than notification.ID; otherwise a stale status/read can erase another browser’s acknowledgment and make that result unread again.

(Based on your team's feedback about preserving unknown newer completion receipts.) [c0c5fc9e-ea64-47bc-9858-055049e42411]

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At internal/consoleserver/completion_reads.go, line 66:

<comment>This cleanup deletes unknown or newer receipts, not just known superseded ones. Preserve receipt values that cannot be proven older than `notification.ID`; otherwise a stale status/read can erase another browser’s acknowledgment and make that result unread again.

(Based on your team's feedback about preserving unknown newer completion receipts.) [c0c5fc9e-ea64-47bc-9858-055049e42411]</comment>

<file context>
@@ -59,6 +62,11 @@ func (s *Server) readSessionCompletion(writer http.ResponseWriter, request *http
 			session.Annotations = map[string]string{}
 		}
+		for annotation, id := range session.Annotations {
+			if strings.HasPrefix(annotation, completionReadAnnotationPrefix) && id != notification.ID {
+				delete(session.Annotations, annotation)
+			}
</file context>