fix(api): handle corrupted data in sandbox stop time#3203
Conversation
PR SummaryLow Risk Overview Reviewed by Cursor Bugbot for commit c119758. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Code Review
This pull request refactors the sbxStopTime function to return the start time when the end time is not after the start time, and updates the unit tests to include a new test suite for corrupt end times. I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
LGTM — small, well-tested fix that collapses corrupt records to a zero-duration analytics event.
Extended reasoning...
Overview
This PR modifies sbxStopTime in packages/api/internal/orchestrator/analytics.go so that any corrupt sandbox record (EndTime not strictly after StartTime, including the zero value) now returns StartTime, producing a zero duration for analytics. Previously such records could fall through to return now, yielding an inflated duration. The test file is reorganized: normal cases stay in TestSbxStopTime, and a new TestSbxStopTime_CorruptEndTime covers the three corrupt shapes (end < start, end == start, zero end).
Security risks
None. This is a pure-function refactor on analytics reporting math; no auth, crypto, input parsing, or external I/O is touched.
Level of scrutiny
Low. The affected function is ~10 lines, has direct unit tests exercising each branch, and only influences PostHog/analytics payloads — not sandbox lifecycle, billing enforcement, or user-visible responses. The behavior change is well-scoped to the corrupt-data path, which is what the PR title and tests both describe.
Other factors
The only reported issue is a typo (bZ → be) in a test comment, already flagged as an inline nit with no runtime impact. No outstanding reviewer comments to address.
Handle a case where EndTime < StartTime < now