Conversation
|
@5LABS is attempting to deploy a commit to the Sim Team on Vercel. A member of the Team first needs to authorize it. |
PR SummaryMedium Risk Overview Introduces a suite of new Adjusts anonymous auth Written by Cursor Bugbot for commit 7de53f3. This will update automatically on new commits. Configure here. |
Greptile SummaryThis PR adds a full Clockify integration to the Sim platform, introducing three new blocks ( Key issues found:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as Sim UI Block
participant Block as ClockifyBlock / ClockifyReportsBlock / ClockifyTimeBlock
participant Executor as Tool Executor
participant API as Clockify REST API
participant ReportsAPI as Clockify Reports API
UI->>Block: Select operation + enter params (apiKey, workspaceId, etc.)
Block->>Executor: tool(params.operation), params(params)
alt Basic operations (get_current_user, get_workspaces, get_users, get_member_profile, get_projects)
Executor->>API: GET /api/v1/... with X-Api-Key header
API-->>Executor: JSON response
Executor-->>UI: { success: true, output: {...} }
end
alt Time entry operations (get_time_entries, get_time_entry, get_in_progress)
Executor->>API: GET /api/v1/workspaces/{id}/... with X-Api-Key header
API-->>Executor: JSON response
Executor-->>UI: { success: true, output: { timeEntries: [...] } }
end
alt Time off / holidays
Executor->>API: POST /api/v1/workspaces/{id}/time-off/requests (⚠ should be GET)
API-->>Executor: JSON response
Executor-->>UI: { success: true, output: { requests: [...] } }
end
alt Report operations (summary, detailed, weekly, attendance)
Executor->>ReportsAPI: POST /v1/workspaces/{id}/reports/{type} with date range + filters
ReportsAPI-->>Executor: JSON with groupOne / totals
Executor-->>UI: { success: true, output: { groups/timeentries/weeks/attendance: [...] } }
end
Last reviewed commit: 7de53f3 |
| export function createAnonymousGetSessionResponse(): AnonymousSession { | ||
| return createAnonymousSession() |
There was a problem hiding this comment.
Unrelated breaking change to anonymous session response shape
The return type was changed from { data: AnonymousSession } to AnonymousSession, which alters the JSON response shape of GET /api/auth/get-session when auth is disabled (isAuthDisabled).
Before, calling NextResponse.json(createAnonymousGetSessionResponse()) returned:
{ "data": { "user": {...}, "session": {...} } }After, it returns the AnonymousSession object directly (no data wrapper).
The existing test in apps/sim/app/api/auth/[...all]/route.test.ts still mocks this function to return the old { data: { user: ... } } shape (line 11–16), so the test continues to pass — but it is now testing a mock that no longer matches the actual implementation. Any client code (e.g. browser session hooks) that reads the data property from this endpoint response will silently break when DISABLE_AUTH is set.
This change appears unrelated to the Clockify feature and should be separated or carefully reviewed for its impact on the anonymous auth flow.
| request: { | ||
| url: (params) => | ||
| `https://api.clockify.me/api/v1/workspaces/${params.workspaceId}/time-off/requests`, | ||
| method: 'POST', |
There was a problem hiding this comment.
POST used to retrieve time-off requests — likely wrong HTTP method
The Clockify REST API uses GET /workspaces/{workspaceId}/time-off/requests for listing time-off requests. This tool uses POST with a JSON body containing start/end, which matches the signature of creating a time-off request, not fetching them.
Using POST on this endpoint will likely return a 405 Method Not Allowed or create unintended records instead of reading them. Date filtering should be passed as query parameters on a GET request:
// Correct approach
request: {
url: (params) => {
const base = `https://api.clockify.me/api/v1/workspaces/${params.workspaceId}/time-off/requests`
const query: string[] = []
if (params.start) query.push(`start=${encodeURIComponent(params.start)}`)
if (params.end) query.push(`end=${encodeURIComponent(params.end)}`)
return query.length > 0 ? `${base}?${query.join('&')}` : base
},
method: 'GET',
headers: (params) => ({
'X-Api-Key': params.apiKey,
}),
},| return { | ||
| success: true, | ||
| output: { | ||
| attendance: data.groupOne || [], |
There was a problem hiding this comment.
Attendance report response may be incorrectly mapped from
groupOne
The Clockify attendance report endpoint (/reports/attendance) returns attendance records under a top-level array (e.g. data.report or similar), not data.groupOne. The groupOne field is specific to the summary and weekly report responses which group entries by dimension.
Using data.groupOne || [] here will silently produce an empty array if the attendance API does not return that field, making the tool appear to succeed with no records even when data exists.
This same mapping is used in report_summary.ts (line 97) and report_weekly.ts (line 97) with the same data.groupOne pattern. Please verify the attendance report API response structure against the Clockify documentation and use the correct top-level key for attendance records.
| url: (params) => | ||
| `https://api.clockify.me/api/v1/workspaces/${params.workspaceId}/time-entries/in-progress`, | ||
| method: 'GET', |
There was a problem hiding this comment.
Non-standard Clockify API endpoint for in-progress timers
The Clockify REST API does not document a GET /workspaces/{workspaceId}/time-entries/in-progress endpoint. The standard approach for finding currently running timers is to query a specific user's time entries with the in-progress query flag:
GET /workspaces/{workspaceId}/user/{userId}/time-entries?in-progress=true
Using the unsupported path may result in a 404 Not Found. If the intent is to fetch in-progress entries across all users in the workspace, the Clockify API does not natively support this in a single call — it would require iterating over users. Please confirm the correct endpoint from the Clockify API documentation before shipping.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| export function createAnonymousGetSessionResponse(): { data: AnonymousSession } { | ||
| return { data: createAnonymousSession() } | ||
| export function createAnonymousGetSessionResponse(): AnonymousSession { | ||
| return createAnonymousSession() |
There was a problem hiding this comment.
Anonymous session response missing data wrapper
High Severity
createAnonymousGetSessionResponse was changed to return AnonymousSession directly instead of { data: AnonymousSession }. The /api/auth/get-session endpoint uses this return value directly in NextResponse.json(...), so the HTTP response body is now { user, session } instead of { data: { user, session } }. The better-auth protocol wraps session data in a data envelope; without it, the client library will fail to recognize the anonymous session and treat the user as unauthenticated. The test in route.test.ts explicitly asserts the { data: ... } wrapper (line 65), but its mock still returns the old format, silently hiding the regression.
|
|
||
| request: { | ||
| url: (params) => | ||
| `https://api.clockify.me/api/v1/workspaces/${params.workspaceId}/time-off/requests`, |
There was a problem hiding this comment.
Time-off tool uses wrong API domain and path
High Severity
The time-off requests tool calls https://api.clockify.me/api/v1/workspaces/{id}/time-off/requests, but the actual Clockify time-off/PTO API lives on a separate subdomain: https://pto.api.clockify.me/v1/workspaces/{id}/requests. Because the base domain is wrong, every invocation of this tool will hit an invalid endpoint and fail entirely.
| } | ||
| } | ||
| return body | ||
| }, |
There was a problem hiding this comment.
All report tools missing required API body fields
High Severity
All four Clockify report tools (report_summary.ts, report_detailed.ts, report_weekly.ts, report_attendance.ts) build a POST body containing only dateRangeStart and dateRangeEnd. The Clockify Reports API requires exportType (e.g., "JSON") as a mandatory field on every report request, and the summary report additionally requires a summaryFilter object while the detailed report requires detailedFilter. Without these fields the API returns a 400 error, making all four report operations completely non-functional.


Summary
Brief description of what this PR does and why.
Fixes #(issue)
Type of Change
Testing
How has this been tested? What should reviewers focus on?
Checklist
Screenshots/Videos