[kernel 1116] browser logging: cdp pipeline#194
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
archandatta
force-pushed
the
archand/kernel-1116/cdp-pipeline
branch
from
1e544a7 to
6d929c8
Compare
archandatta
force-pushed
the
archand/kernel-1116/cdp-pipeline
branch
from
6d929c8 to
76d837f
Compare
…ies and fix delimiter in CategoryFor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| logger.FromContext(r.Context()).Error("failed to start CDP monitor", "err", err) | ||
| http.Error(w, "failed to start capture", http.StatusInternalServerError) | ||
| return | ||
| } |
There was a problem hiding this comment.
Session ID changed before monitor restart causes misattribution
Medium Severity
captureSession.Start(newID) is called before cdpMonitor.Start(), which atomically changes the session ID while the old monitor may still be publishing events. Since captureSessionID is an atomic.Pointer readable without holding monitorMu, any events published by the old monitor's goroutines during the transition window will be incorrectly tagged with the new session ID. The session ID change needs to happen after the old monitor is fully stopped, not before.
| nextSeq := afterSeq + 1 | ||
| if afterSeq == 0 { | ||
| nextSeq = 1 | ||
| } |
There was a problem hiding this comment.
Redundant branch duplicates general-case logic in NewReader
Low Severity
The special case if afterSeq == 0 { nextSeq = 1 } is redundant because the general computation nextSeq := afterSeq + 1 already produces 1 when afterSeq is 0. This no-op branch adds cognitive overhead and may mislead future readers into thinking the afterSeq == 0 case is somehow special.
1 participant
Note
Medium Risk
Introduces new HTTP endpoints and lifecycle management for an event-capture pipeline, adding concurrency/locking and new background monitor shutdown behavior. Risk is moderate because it touches server startup/shutdown and writes event logs to disk, but the CDP monitor implementation is currently minimal.
Overview
Adds an event-capture pipeline to the API service by wiring in a new
events.CaptureSessionand acdpmonitor.Monitor, including lifecycle management inApiService.NewandShutdown.Exposes new non-OpenAPI endpoints
POST /events/startandPOST /events/stopto (re)seed a fresh capture session ID and start/stop the CDP monitor under a mutex.Refactors
events.CaptureSessionto be startable viaStart(sessionID)(constructor no longer takes an ID), switches timestamps to milliseconds, makes sequencing atomic, and addsevents.CategoryForplus a ring-buffer reader start-position fix; tests are updated accordingly.Written by Cursor Bugbot for commit ad2dd63. This will update automatically on new commits. Configure here.