feat(toggl): add Toggl Track integration - #672
Conversation
|
@Agam00 is attempting to deploy a commit to the corsair Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds a Toggl Track provider with API v9 support, typed endpoint schemas, CRUD operations, API-key authentication, error handling, webhook tenant matching, persistence, package configuration, integration tests, and provider registration. ChangesToggl Track provider
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CorsairPlugin
participant TogglEndpoint
participant makeTogglRequest
participant TogglAPI
CorsairPlugin->>TogglEndpoint: Invoke typed endpoint
TogglEndpoint->>makeTogglRequest: Send path, method, body, and query
makeTogglRequest->>TogglAPI: Send Basic-authenticated API request
TogglAPI-->>makeTogglRequest: Return JSON response or API error
makeTogglRequest-->>TogglEndpoint: Return parsed result or classified error
TogglEndpoint-->>CorsairPlugin: Return typed output
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe follow-up adds the missing endpoint-level tests, credential redaction, and entity-cache synchronization requested in prior review.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains from the previously reported issues. The synthetic test credential, profile redaction, endpoint coverage, alternate-list cache synchronization, and archive cache handling now address the prior findings without leaving a blocking residual failure. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
Caller["Corsair caller"] --> Endpoint["Toggl endpoint wrapper"]
Endpoint --> API["Toggl Track API"]
API --> Redaction["Credential redaction"]
Redaction --> Cache["Entity cache synchronization"]
Cache --> Result["Safe endpoint result"]
Reviews (7): Last reviewed commit: "fix(toggl): use country_id for subdivisi..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Plugin PR scorecard —
|
| Check | Status | Notes |
|---|---|---|
| R1 — Scope: plugin files only | ✅ | |
| R2 — Tests with assertions | ✅ | |
| R3 — Description complete | ✅ | |
| R3 — Linked issue / claim | ✅ | |
| R4 — Demo video / recording | ✅ |
Rules: PLUGIN_PR_RULES.md · re-runs on every push
|
Hey @Agam00, thanks for the contribution! 🏴☠️ Before a maintainer reviews, please fix the items below — the review re-runs automatically on your next push. Must fix
Rule Used: Flag any use of eval, new Function(), or execution... (source)
Knowledge Base Used: The provider-plugin package pattern
Knowledge Base Used: The provider-plugin package pattern
Rule Used: Flag Knowledge Base Used: The provider-plugin package pattern Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! If anything remains after your next push, a bot commit will clean it up; a maintainer always does the final review and merge. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
packages/toggl/endpoints/types.ts (3)
458-459: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the comment with the optional field.
created_withis optional here.endpoints/time-entries.tsline 85 supplies the default'corsair'. Update the comment to state that the endpoint fills the default.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/toggl/endpoints/types.ts` around lines 458 - 459, Update the JSDoc comment for the created_with field in the relevant schema to indicate that it is optional and the endpoint supplies the default value “corsair,” keeping the z.string().optional() definition unchanged.
421-429: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueEnforce the documented
start_date/end_datepairing.The comment states that
start_datemust be paired withend_date. The schema does not enforce this. Add a refinement so the mismatch fails locally instead of at the API.♻️ Proposed change
- meta: z.boolean().optional(), -}); + meta: z.boolean().optional(), +}).refine( + (v) => (v.start_date === undefined) === (v.end_date === undefined), + { error: 'start_date and end_date must be provided together' }, +);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/toggl/endpoints/types.ts` around lines 421 - 429, Update TimeEntriesListInputSchema to add an object-level refinement requiring start_date and end_date to either both be provided or both be absent. Ensure validation rejects either mismatched combination locally while preserving all existing field validation.
176-182: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueUse
z.email()forMeUpdateInputSchema.email.The field currently accepts any string. Zod 4.4.3 supports
z.email()and rejects invalid addresses during input validation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/toggl/endpoints/types.ts` around lines 176 - 182, Update the email field in MeUpdateInputSchema to use Zod’s z.email() validator while preserving its optional behavior, so invalid email addresses are rejected during input validation.packages/toggl/client.ts (2)
20-35: 🚀 Performance & Scalability | 🔵 TrivialConsider proactive pacing in addition to retries.
Toggl allows roughly one request per second per token. This config reacts after a 429 only. With five retries and a 2x multiplier, one call can sleep about 31 seconds before it fails.
integration.test.tsadds its ownpace()helper, which shows the gap. A shared token-bucket limiter in front ofrequestwould smooth bursts and reduce 429 responses.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/toggl/client.ts` around lines 20 - 35, Add proactive token-bucket pacing before requests are issued, reusing the shared rate-limiting path around request rather than relying only on TOGGL_RATE_LIMIT_CONFIG retries. Configure it for roughly one request per second per API token, ensure concurrent calls share the limiter, and preserve the existing 429 retry and Retry-After handling.
8-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
TogglAPIErrorclass.No repository code constructs or throws
TogglAPIError.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/toggl/client.ts` around lines 8 - 16, Remove the unused TogglAPIError class from the client module, including its constructor and related definition, while leaving the surrounding API client code unchanged.packages/toggl/endpoints/clients.ts (1)
7-17: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winList handlers declare a non-nullable transport result but guard against null. Each list handler passes the plugin output type as the
makeTogglRequestgeneric. That type excludesnull, while Toggl returnsnullfor an empty collection. The type checker therefore treats each?? []guard as dead code, and a later cleanup could remove it and reintroduce a null return. Add| nullto the generic at each site.
packages/toggl/endpoints/clients.ts#L7-L17: change the generic toTogglEndpointOutputs['clientsList'] | null.packages/toggl/endpoints/projects.ts#L7-L21: change the generic toTogglEndpointOutputs['projectsList'] | null.packages/toggl/endpoints/tasks.ts#L7-L13: change the generic toTogglEndpointOutputs['tasksList'] | null.packages/toggl/endpoints/tags.ts#L7-L13: change the generic toTogglEndpointOutputs['tagsList'] | null.packages/toggl/endpoints/time-entries.ts#L7-L20: change the generic toTogglEndpointOutputs['timeEntriesList'] | null.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/toggl/endpoints/clients.ts` around lines 7 - 17, Update the makeTogglRequest generic in the list handlers to include null, preserving each existing null-to-empty-array fallback: packages/toggl/endpoints/clients.ts lines 7-17 use clientsList | null; packages/toggl/endpoints/projects.ts lines 7-21 use projectsList | null; packages/toggl/endpoints/tasks.ts lines 7-13 use tasksList | null; packages/toggl/endpoints/tags.ts lines 7-13 use tagsList | null; and packages/toggl/endpoints/time-entries.ts lines 7-20 use timeEntriesList | null.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/toggl/endpoints/me.ts`:
- Line 36: Update the logEventFromContext call in the me update handler to
exclude profile values from the payload. Log only the update operation and the
names of fields being changed, without spreading input or persisting
email/fullname values.
In `@packages/toggl/endpoints/time-entries.ts`:
- Around line 89-128: Update the create and update handlers in
packages/toggl/endpoints/time-entries.ts (lines 89-128) so logEventFromContext
records only workspace_id, project_id, task_id, and the time-entry id, excluding
description and other free-text input fields. Also update the create and update
handlers in packages/toggl/endpoints/clients.ts (lines 28-80) to record only
workspace_id and client_id, excluding name; replace each full-input event
payload with these identifier-only fields.
In `@packages/toggl/error-handlers.ts`:
- Around line 38-71: Update AUTH_ERROR and PERMISSION_ERROR in
packages/toggl/error-handlers.ts (lines 38-71) to inspect ApiError.body for
Toggl’s invalid-token condition, classifying matching 403 responses as
authentication failures and excluding them from permission failures. Add a 403
ApiError invalid-token fixture and assertions in packages/toggl/client.test.ts
(lines 109-122) confirming it matches AUTH_ERROR but not PERMISSION_ERROR.
In `@packages/toggl/integration.test.ts`:
- Around line 75-100: Make remote-resource cleanup failure-safe across
packages/toggl/integration.test.ts: in lines 75-100, 103-126, and 129-151, track
each created client, project, and tag and delete it from finally or equivalent
cleanup even when later requests or assertions fail; in lines 154-196, stop and
delete the created time entry in the same failure-safe cleanup; update the
cleanup guarantee at line 13 to reflect this behavior.
- Around line 208-212: Update the unknown-resource test around makeTogglRequest
to use a guaranteed-unresolvable Toggl route instead of clients/1, then assert
the specific expected not-found error rather than only checking that some error
is thrown.
In `@packages/toggl/schema/database.ts`:
- Around line 28-34: Update the Toggl client persistence flow using
TogglClientEntity and db.clients.upsertByEntityId to map the API response’s wid
field into workspace_id before schema parsing or persistence, preserving the
workspace relation instead of allowing wid to be stripped as unknown.
---
Nitpick comments:
In `@packages/toggl/client.ts`:
- Around line 20-35: Add proactive token-bucket pacing before requests are
issued, reusing the shared rate-limiting path around request rather than relying
only on TOGGL_RATE_LIMIT_CONFIG retries. Configure it for roughly one request
per second per API token, ensure concurrent calls share the limiter, and
preserve the existing 429 retry and Retry-After handling.
- Around line 8-16: Remove the unused TogglAPIError class from the client
module, including its constructor and related definition, while leaving the
surrounding API client code unchanged.
In `@packages/toggl/endpoints/clients.ts`:
- Around line 7-17: Update the makeTogglRequest generic in the list handlers to
include null, preserving each existing null-to-empty-array fallback:
packages/toggl/endpoints/clients.ts lines 7-17 use clientsList | null;
packages/toggl/endpoints/projects.ts lines 7-21 use projectsList | null;
packages/toggl/endpoints/tasks.ts lines 7-13 use tasksList | null;
packages/toggl/endpoints/tags.ts lines 7-13 use tagsList | null; and
packages/toggl/endpoints/time-entries.ts lines 7-20 use timeEntriesList | null.
In `@packages/toggl/endpoints/types.ts`:
- Around line 458-459: Update the JSDoc comment for the created_with field in
the relevant schema to indicate that it is optional and the endpoint supplies
the default value “corsair,” keeping the z.string().optional() definition
unchanged.
- Around line 421-429: Update TimeEntriesListInputSchema to add an object-level
refinement requiring start_date and end_date to either both be provided or both
be absent. Ensure validation rejects either mismatched combination locally while
preserving all existing field validation.
- Around line 176-182: Update the email field in MeUpdateInputSchema to use
Zod’s z.email() validator while preserving its optional behavior, so invalid
email addresses are rejected during input validation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ad5537d6-3231-4545-810b-0141d8643439
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
packages/corsair/core/constants.tspackages/toggl/client.test.tspackages/toggl/client.tspackages/toggl/endpoints/clients.tspackages/toggl/endpoints/index.tspackages/toggl/endpoints/me.tspackages/toggl/endpoints/organizations.tspackages/toggl/endpoints/projects.tspackages/toggl/endpoints/tags.tspackages/toggl/endpoints/tasks.tspackages/toggl/endpoints/time-entries.tspackages/toggl/endpoints/types.tspackages/toggl/endpoints/workspaces.tspackages/toggl/error-handlers.tspackages/toggl/index.tspackages/toggl/integration.test.tspackages/toggl/jest.config.cjspackages/toggl/package.jsonpackages/toggl/schema.test.tspackages/toggl/schema/database.tspackages/toggl/schema/index.tspackages/toggl/tsconfig.jsonpackages/toggl/tsup.config.tspackages/toggl/webhooks/index.tspackages/toggl/webhooks/oauth-tenant-link.tspackages/toggl/webhooks/tenant-matcher.tspackages/toggl/webhooks/types.ts
| it('surfaces a clear error for an unknown resource', async () => { | ||
| await pace(); | ||
| await expect( | ||
| makeTogglRequest<unknown>(`workspaces/${workspaceId}/clients/1`, token), | ||
| ).rejects.toThrow(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a guaranteed invalid route.
Client ID 1 can exist in the selected workspace. The request can then succeed and make this test fail. Use a route that Toggl cannot resolve, and assert the expected not-found error.
Proposed fix
- makeTogglRequest<unknown>(`workspaces/${workspaceId}/clients/1`, token),
+ makeTogglRequest<unknown>('corsair-test-route-does-not-exist', token),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('surfaces a clear error for an unknown resource', async () => { | |
| await pace(); | |
| await expect( | |
| makeTogglRequest<unknown>(`workspaces/${workspaceId}/clients/1`, token), | |
| ).rejects.toThrow(); | |
| it('surfaces a clear error for an unknown resource', async () => { | |
| await pace(); | |
| await expect( | |
| makeTogglRequest<unknown>('corsair-test-route-does-not-exist', token), | |
| ).rejects.toThrow(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/toggl/integration.test.ts` around lines 208 - 212, Update the
unknown-resource test around makeTogglRequest to use a guaranteed-unresolvable
Toggl route instead of clients/1, then assert the specific expected not-found
error rather than only checking that some error is thrown.
| export const TogglClientEntity = z.object({ | ||
| id: z.number(), | ||
| workspace_id: z.number().nullable().optional(), | ||
| name: z.string(), | ||
| archived: z.boolean().nullable().optional(), | ||
| at: z.coerce.date().nullable().optional(), | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the client persistence path and verify that it maps Toggl's `wid`.
ast-grep outline packages/toggl --items all --type function
rg -n -C 8 '\bTogglClientEntity\b|\bwid\b|\bworkspace_id\b' packages/togglRepository: corsairdev/corsair
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Toggl package files ---'
git ls-files packages/toggl
printf '%s\n' '--- Toggl schema and persistence-related references ---'
rg -n -C 5 'TogglSchema|entities:|database|persist|parse\(|safeParse\(|clientsList|clientsGet' packages/toggl packages/corsair/core --glob '*.{ts,tsx,js,jsx}' | head -n 500
printf '%s\n' '--- Relevant source sections ---'
sed -n '1,180p' packages/toggl/endpoints/clients.ts
sed -n '1,130p' packages/toggl/schema/index.ts
sed -n '1,90p' packages/toggl/schema/database.ts
sed -n '80,110p' packages/toggl/endpoints/types.tsRepository: corsairdev/corsair
Length of output: 39421
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Entity client construction and schema usage ---'
rg -n -C 8 'createKyselyEntityClient|PluginEntityClient|schema\.entities|entities\[' packages/corsair/core packages/corsair/db --glob '*.{ts,tsx}' | head -n 600
printf '%s\n' '--- Complete relevant core client sections ---'
sed -n '1,180p' packages/corsair/core/client/index.ts
sed -n '1,260p' packages/corsair/db/kysely/orm.ts
printf '%s\n' '--- All Toggl entity-client references ---'
rg -n -C 8 'entities\.(workspaces|clients|projects|tags)|\.clients\b|TogglClientEntity' packages --glob '*.{ts,tsx}' | head -n 500Repository: corsairdev/corsair
Length of output: 37766
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Entity write methods ---'
sed -n '239,560p' packages/corsair/db/kysely/orm.ts
printf '%s\n' '--- Entity client write type definitions ---'
sed -n '1090,1215p' packages/corsair/db/orm.ts
printf '%s\n' '--- Entity write call sites and Toggl database usage ---'
rg -n -C 6 '\.(create|upsert|update)\s*\(' packages --glob '*.{ts,tsx}' | rg -C 4 'db|entity|Toggl|clients|projects|tags|workspaces' | head -n 500Repository: corsairdev/corsair
Length of output: 24847
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const zodPath = require.resolve('zod');
const { z } = require(zodPath);
const TogglClientEntity = z.object({
id: z.number(),
workspace_id: z.number().nullable().optional(),
name: z.string(),
archived: z.boolean().nullable().optional(),
at: z.coerce.date().nullable().optional(),
});
const input = {
id: 69027594,
wid: 21597802,
name: 'Acme Corp',
archived: false,
at: '2026-08-12T09:50:41+00:00',
};
const parsed = TogglClientEntity.parse(input);
console.log(JSON.stringify({
zodPath,
parsedKeys: Object.keys(parsed),
parsed,
widPreserved: Object.prototype.hasOwnProperty.call(parsed, 'wid'),
workspaceId: parsed.workspace_id ?? null,
}));
JSRepository: corsairdev/corsair
Length of output: 944
🌐 Web query:
Zod official documentation object schemas unknown keys default strip behavior
💡 Result:
In Zod, the default behavior for object schemas when encountering unknown keys (keys not explicitly defined in the schema) is to strip them from the parsed result [1][2][3]. This means if you parse an input object containing extra keys, Zod will return a new object containing only the keys specified in your schema [1][3]. Example of default behavior: const User = z.object({ name: z.string }); User.parse({ name: "Alice", age: 30 }); // Result: { name: "Alice" } To change this behavior, you can use the following methods on your object schema:.strict Disallows unknown keys by throwing a ZodError if any are present during parsing [2][3][4]..passthrough (or.loose in newer versions) Allows unknown keys to pass through the parsed result without modification or validation [2][3][4]..catchall(schema) Validates all unknown keys against a provided Zod schema [5][4]. When.catchall is used, it takes precedence over.strip,.strict, or.passthrough, and all keys are considered "known" [2][6][7].
Citations:
- 1: https://zod.dev/api
- 2: https://v3.zod.dev/
- 3: Confused by strict / nonstrict / parse behaviour & documentation colinhacks/zod#490
- 4: https://colinhacks-zod.mintlify.app/api/complex/object
- 5: https://github.com/colinhacks/zod/blob/c7805073/packages/docs/content/api.mdx
- 6: https://github.com/colinhacks/zod/blob/9575cfecbabeb149a4991f931cf24f685ea8c55e/README.md
- 7: https://github.com/colinhacks/zod/blob/c8ce27e8c0378856964cf8892c6102bde63fa0cb/README.md
Map wid to workspace_id before persistence.
db.clients.upsertByEntityId parses data with TogglClientEntity, which strips the API response’s unknown wid field. Directly persisting a TogglClient therefore loses the workspace relation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/toggl/schema/database.ts` around lines 28 - 34, Update the Toggl
client persistence flow using TogglClientEntity and db.clients.upsertByEntityId
to map the API response’s wid field into workspace_id before schema parsing or
persistence, preserving the workspace relation instead of allowing wid to be
stripped as unknown.
d562590 to
853ba2b
Compare
|
@greptile review |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/toggl/endpoints/clients.ts`:
- Around line 73-80: The clients update flow must separate detail updates from
archive state changes: in update, send only the client details through the PUT
request, then call the documented POST archive or restore operation when
input.archived requests a state change. Update the tests in
packages/toggl/endpoints.test.ts lines 246-256 to cover both archive and restore
routes rather than asserting archived in the PUT body.
In `@packages/toggl/endpoints/projects.ts`:
- Around line 8-20: Update TagsListInputSchema and Tags.list in
packages/toggl/endpoints/tags.ts to accept and forward page, per_page, and
search as query parameters alongside workspace_id. Add or update generated-query
coverage in packages/toggl/endpoints.test.ts at lines 271-279 and 393-403 to
verify all three parameters are included; packages/toggl/endpoints/projects.ts
lines 8-20 is the reference pattern and requires no direct change.
In `@packages/toggl/endpoints/types.ts`:
- Around line 447-455: Update TimeEntriesCreateInputSchema and the corresponding
update schema in packages/toggl/endpoints/types.ts at lines 447-455 and 470-476:
validate start and optional stop with z.iso.datetime(), and require duration to
be an integer via .int() in both mutation schemas.
- Around line 426-434: Update TimeEntriesListInputSchema so start_date,
end_date, and before validate ISO date or RFC3339 datetime strings with offsets
via the appropriate z.iso validators, while preserving their optional status. Do
not add cross-field validation requiring start_date and end_date to be supplied
together.
In `@packages/toggl/endpoints/workspaces.ts`:
- Around line 33-49: Remove api_token from workspace response objects by
introducing one shared sanitizer and applying it in workspaces.get,
workspaces.list, and workspaces.update before returning results. Preserve all
other workspace fields and add regression coverage for responses containing
api_token.
In `@packages/toggl/integration.test.ts`:
- Around line 80-85: Update the integration test setup around beforeAll and the
authenticated-user test so the first makeTogglRequest('me', token) call is
delayed by the declared one-second request margin when workspace discovery has
sent the workspaces request. Preserve existing timing behavior when
TOGGL_WORKSPACE_ID is already provided.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f2b9038e-1be5-460a-8295-17bd04ffed80
📒 Files selected for processing (16)
packages/toggl/client.test.tspackages/toggl/endpoints.test.tspackages/toggl/endpoints/clients.tspackages/toggl/endpoints/logging.tspackages/toggl/endpoints/me.tspackages/toggl/endpoints/organizations.tspackages/toggl/endpoints/persist.tspackages/toggl/endpoints/projects.tspackages/toggl/endpoints/tags.tspackages/toggl/endpoints/tasks.tspackages/toggl/endpoints/time-entries.tspackages/toggl/endpoints/types.tspackages/toggl/endpoints/workspaces.tspackages/toggl/error-handlers.tspackages/toggl/integration.test.tspackages/toggl/schema.test.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/toggl/endpoints/organizations.ts
- packages/toggl/client.test.ts
- packages/toggl/schema.test.ts
- packages/toggl/endpoints/tasks.ts
- packages/toggl/endpoints/time-entries.ts
- packages/toggl/error-handlers.ts
| export const list: TogglEndpoints['projectsList'] = async (ctx, input) => { | ||
| const result = await makeTogglRequest<TogglEndpointOutputs['projectsList']>( | ||
| `workspaces/${input.workspace_id}/projects`, | ||
| ctx.key, | ||
| { | ||
| method: 'GET', | ||
| query: { | ||
| active: input.active, | ||
| name: input.name, | ||
| page: input.page, | ||
| per_page: input.per_page, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- relevant files ---'
git ls-files 'packages/toggl/**' '.github/PLUGIN_PR_RULES.md' | sed -n '1,160p'
printf '%s\n' '--- endpoint outlines ---'
ast-grep outline packages/toggl/endpoints/projects.ts
ast-grep outline packages/toggl/endpoints/tags.ts
printf '%s\n' '--- project endpoint ---'
cat -n packages/toggl/endpoints/projects.ts
printf '%s\n' '--- tag endpoint ---'
cat -n packages/toggl/endpoints/tags.ts
printf '%s\n' '--- shared request and schemas ---'
rg -n -C 3 'makeTogglRequest|projectsList|tagsList|start_project_id|per_page|search' packages/toggl
printf '%s\n' '--- relevant tests ---'
sed -n '220,310p' packages/toggl/endpoints.test.ts
sed -n '360,425p' packages/toggl/endpoints.test.tsRepository: corsairdev/corsair
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- endpoint types: project and tag schemas ---'
sed -n '270,330p' packages/toggl/endpoints/types.ts
rg -n -A 18 -B 5 'TagsListInputSchema|ProjectsListOutput|TagsListOutput|ProjectsListInputSchema|TagsListInput' packages/toggl/endpoints/types.ts
printf '%s\n' '--- request implementation ---'
sed -n '1,135p' packages/toggl/client.ts
printf '%s\n' '--- focused endpoint tests ---'
sed -n '250,290p' packages/toggl/endpoints.test.ts
sed -n '380,415p' packages/toggl/endpoints.test.ts
printf '%s\n' '--- git state ---'
git diff --stat
git status --short
printf '%s\n' '--- project/tag references outside focused files ---'
rg -n -g '*.ts' 'start_project_id|projectsList|tagsList|/paginated|page:|per_page:|search:' packages/toggl | sed -n '1,220p'
printf '%s\n' '--- query serialization verifier ---'
python3 - <<'PY'
from urllib.parse import urlencode, urljoin
queries = [
{"active": True, "name": None, "page": 2, "per_page": 50},
{"active": None, "name": None, "page": None, "per_page": None},
]
for query in queries:
# Model URLSearchParams-style filtering used by typical request serializers:
filtered = {k: v for k, v in query.items() if v is not None}
print(urlencode(filtered))
PYRepository: corsairdev/corsair
Length of output: 14256
🌐 Web query:
Toggl Track API v9 projects list paginated start_project_id page per_page official documentation
💡 Result:
In the Toggl Track API v9, project listing is handled differently depending on whether you use standard list endpoints or specifically designed paginated endpoints. For user-specific projects, the API provides a dedicated paginated endpoint [1][2]: Endpoint: GET /api/v9/me/projects/paginated [2] Supported query parameters for this endpoint include [1][2]: - start_project_id: (integer, optional) The project ID to use as an offset to resume pagination [1][2]. - per_page: (integer, optional) The number of items to return per page (default is 201) [1][2]. - since: (integer, optional) Retrieves projects created, modified, or deleted since this Unix timestamp [1][2]. For workspace-level projects, the standard endpoint is typically: Endpoint: GET /api/v9/workspaces/{workspace_id}/projects [3][4] The workspace projects endpoint uses a page-based system rather than a project ID offset [3][4]: - page: (integer, required) The page number to retrieve [3][4]. - per_page: (integer, optional) The number of items per page (default is 151, maximum 200) [3][4]. Note that for some general project list requests, you can also append /paginated to a base URL to enable pagination features [5][6]. Always refer to the specific documentation for the resource you are querying, as parameter requirements (such as "required" versus "optional" status) can vary between the workspace and user-level endpoints [1][3].
Citations:
- 1: https://engineering.toggl.com/docs/api/me/
- 2: https://community.toggl.com/t/me-endpoints-get-projectspaginated/169/1
- 3: https://engineering.toggl.com/docs/track/api/projects/
- 4: https://community.toggl.com/t/projects-endpoints-get-workspace-projects/370
- 5: https://engineering.toggl.com/docs/projects/
- 6: https://engineering.toggl.com/docs/track/projects/
🌐 Web query:
Toggl Track API v9 workspace tags list page per_page search official documentation
💡 Result:
The Toggl Track API v9 endpoint for listing tags in a workspace supports pagination and search through standard query parameters. Endpoint: GET https://api.track.toggl.com/api/v9/workspaces/{workspace_id}/tags [1][2] Query Parameters: - page (integer, optional): Specifies the page number to retrieve [1][3]. - per_page (integer, optional): Specifies the number of items to return per page [1][3]. - search (string, optional): Allows filtering/searching the tags by name [1][3]. Example Request: To retrieve tags for a specific workspace with pagination and a search filter, you can append the parameters to the URL: curl -X GET "https://api.track.toggl.com/api/v9/workspaces/{workspace_id}/tags?page=1&per_page=50&search=example" \ -H "Accept: application/json" [3] This endpoint returns a 200 OK response with an array of tag objects [1][3]. Ensure that the {workspace_id} is replaced with the numeric ID of the target workspace [1][4].
Citations:
- 1: https://engineering.toggl.com/docs/api/tags/
- 2: https://engineering.toggl.com/docs/track/api/tags/
- 3: https://www.withone.ai/knowledge/toggl-track/conn_mod_def%3A%3AGLEuCxNpG7A%3A%3AOWKVNPDJQ6K-bzbWoDb_nA
- 4: https://community.toggl.com/t/tags-endpoints-get-tags/384
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- shared query serialization sources ---'
rg -n -C 5 'URLSearchParams|query.*Object|Object\.entries\(.*query|query:' packages --glob '*.{ts,tsx,js,jsx}' | sed -n '1,240p'
printf '%s\n' '--- tag list conventions in repository ---'
rg -n -C 5 'search: input\.|page: input\.|per_page: input\.|auditPayload\(input.*tags|tags\.list' packages --glob '*.{ts,tsx}' | sed -n '1,240p'
printf '%s\n' '--- source-backed contract verifier ---'
python3 - <<'PY'
from pathlib import Path
import re
from urllib.parse import urlencode
types = Path("packages/toggl/endpoints/types.ts").read_text()
tags = Path("packages/toggl/endpoints/tags.ts").read_text()
projects = Path("packages/toggl/endpoints/projects.ts").read_text()
tag_schema = re.search(
r"const TagsListInputSchema = z\.object\(\{(?P<body>.*?)\n\}\);",
types,
re.S,
)
assert tag_schema, "TagsListInputSchema not found"
tag_fields = set(re.findall(r"^\s*([A-Za-z_][A-Za-z0-9_]*)\s*:", tag_schema.group("body"), re.M))
print("tag_input_fields:", sorted(tag_fields))
print("tag_query_present:", "query:" in tags)
print("project_workspace_route:", re.search(
r"`workspaces/\$\{input\.workspace_id\}/projects`", projects
) is not None)
print("project_uses_start_project_id:", "start_project_id" in projects)
assert tag_fields == {"workspace_id"}
assert "query:" not in tags
assert "start_project_id" not in projects
params = {"page": 2, "per_page": 50, "search": "billable"}
print("expected_tag_query:", urlencode(params))
PYRepository: corsairdev/corsair
Length of output: 28744
🌐 Web query:
Toggl Track official API v9 workspace projects endpoint page per_page not paginated start_project_id
💡 Result:
In the Toggl Track API v9, the standard workspace projects endpoint (GET /workspaces/{workspace_id}/projects) does not support cursor-based pagination using start_project_id, nor does it typically use page-based parameters like per_page in the same manner as paginated endpoints [1][2]. For scenarios requiring pagination, Toggl Track provides dedicated paginated endpoints. Key distinctions are outlined below: 1. Standard Workspace Projects Endpoint: The endpoint GET /api/v9/workspaces/{workspace_id}/projects returns a list of projects for a given workspace [1][2]. While official documentation for this specific endpoint lists parameters like page and per_page (with a default of 151 and a maximum of 200) [3][2], it does not support the start_project_id parameter [3][2]. 2. Paginated Endpoints (e.g., /me/projects/paginated): When you require more advanced pagination, such as cursor-based navigation, you should use endpoints that specifically include /paginated in the path (e.g., GET /api/v9/me/projects/paginated) [1][4]. These endpoints explicitly support: - start_project_id: A query parameter used to resume pagination from a specific project ID [4][5]. - per_page: A query parameter to define the number of items per page (default is 201 for these endpoints) [4][6]. If you are working with workspace-level projects and require cursor-based pagination, ensure you are utilizing an endpoint designed for pagination, as the base workspace projects endpoint is not structured for this advanced pagination method [1][7].
Citations:
- 1: https://engineering.toggl.com/docs/track/projects/
- 2: https://engineering.toggl.com/docs/track/api/projects/
- 3: https://community.toggl.com/t/projects-endpoints-get-workspace-projects/370
- 4: https://engineering.toggl.com/docs/track/api/me/
- 5: https://engineering.toggl.com/docs/api/me/
- 6: https://community.toggl.com/t/me-endpoints-get-projectspaginated/169/1
- 7: https://engineering.toggl.com/docs/projects/
Forward pagination and search parameters for tag listing.
TagsListInputSchema accepts only workspace_id, and Tags.list sends no query parameters. Add page, per_page, and search to the input schema, forward them, and test the generated query.
📍 Affects 3 files
packages/toggl/endpoints/projects.ts#L8-L20(this comment)packages/toggl/endpoints/tags.ts#L8-L13packages/toggl/endpoints.test.ts#L271-L279packages/toggl/endpoints.test.ts#L393-L403
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/toggl/endpoints/projects.ts` around lines 8 - 20, Update
TagsListInputSchema and Tags.list in packages/toggl/endpoints/tags.ts to accept
and forward page, per_page, and search as query parameters alongside
workspace_id. Add or update generated-query coverage in
packages/toggl/endpoints.test.ts at lines 271-279 and 393-403 to verify all
three parameters are included; packages/toggl/endpoints/projects.ts lines 8-20
is the reference pattern and requires no direct change.
| export const get: TogglEndpoints['workspacesGet'] = async (ctx, input) => { | ||
| const result = await makeTogglRequest<TogglEndpointOutputs['workspacesGet']>( | ||
| `workspaces/${input.workspace_id}`, | ||
| ctx.key, | ||
| { method: 'GET' }, | ||
| ); | ||
|
|
||
| await cacheWorkspace(ctx.db.workspaces, result); | ||
|
|
||
| await logEventFromContext( | ||
| ctx, | ||
| 'toggl.workspaces.get', | ||
| auditPayload(input, ['workspace_id']), | ||
| 'completed', | ||
| ); | ||
| return result; | ||
| }; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target file ---'
cat -n packages/toggl/endpoints/workspaces.ts
printf '%s\n' '--- sanitizer and related workspace handlers ---'
rg -n -C 5 'cacheWorkspace|api_token|workspacesGet|Me\.get|workspaces' packages/toggl
printf '%s\n' '--- package files and tests ---'
git ls-files packages/toggl | sed -n '1,200p'Repository: corsairdev/corsair
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- workspace schemas and endpoint output types ---'
cat -n packages/toggl/schema/api.ts 2>/dev/null || true
cat -n packages/toggl/endpoints/types.ts
rg -n -C 8 'TogglWorkspaceSchema|workspacesGet|workspacesList|TogglEndpointOutputSchemas' packages/toggl/schema packages/toggl/endpoints packages/toggl/index.ts
printf '%s\n' '--- me implementation ---'
fd -i 'me*.ts' packages/toggl --exec sh -c 'echo "--- $1"; cat -n "$1"' sh {}
printf '%s\n' '--- workspace persistence ---'
cat -n packages/toggl/endpoints/persist.ts
printf '%s\n' '--- focused endpoint test setup and workspace tests ---'
sed -n '1,190p' packages/toggl/endpoints.test.tsRepository: corsairdev/corsair
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- me endpoint implementation ---'
rg -l 'api_token|export const get.*Me|meGet' packages/toggl/endpoints --glob '*.ts' | while read -r f; do
echo "--- $f"
cat -n "$f"
done
printf '%s\n' '--- focused workspace test section ---'
sed -n '1,190p' packages/toggl/endpoints.test.ts
printf '%s\n' '--- output-schema validation and endpoint invocation ---'
rg -n -C 6 'output\.parse|output\.safeParse|OutputSchemas|endpointSchemas|schema\.parse|safeParse' packages/corsair packages --glob '*.{ts,tsx}' \
| rg -v 'node_modules|dist|build' | sed -n '1,260p'Repository: corsairdev/corsair
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- core endpoint machinery ---'
git ls-files packages/corsair/core | sed -n '1,200p'
rg -n -C 8 'endpointSchemas|outputSchema|\.output|safeParse|parse\(result|parse\(response' packages/corsair/core packages/corsair --glob '*.{ts,tsx}' \
| sed -n '1,320p'
printf '%s\n' '--- plugin endpoint binding and direct exports ---'
rg -n -C 6 'bind.*Endpoint|endpointsNested|plugin\.endpoints|endpointSchemas' packages/corsair packages/toggl --glob '*.{ts,tsx}' \
| sed -n '1,320p'Repository: corsairdev/corsair
Length of output: 41243
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- endpoint binding implementation ---'
sed -n '1,380p' packages/corsair/core/endpoints/bind.ts
printf '%s\n' '--- plugin client endpoint context and returned endpoint tree ---'
sed -n '460,555p' packages/corsair/core/client/index.tsRepository: corsairdev/corsair
Length of output: 14683
🌐 Web query:
Toggl Track API v9 workspaces response api_token
💡 Result:
In the Toggl Track API v9, the api_token field is included in the response for workspace-related endpoints, but it is explicitly marked as deprecated [1][2][3][4][5]. As it is a deprecated field, it should not be relied upon for current or future integrations [1][5].
Citations:
- 1: https://engineering.toggl.com/docs/api/workspaces/
- 2: https://community.toggl.com/t/me-endpoints-get-workspaces/174
- 3: https://community.toggl.com/t/workspaces-endpoints-get-workspaces-containing-search-term/339
- 4: https://community.toggl.com/t/workspaces-endpoints-put-update-workspace/341
- 5: https://community.toggl.com/t/workspaces-endpoints-get-workspace-data/340
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Toggl request response handling ---'
sed -n '1,180p' packages/toggl/client.ts
printf '%s\n' '--- read-only source invariant check ---'
python3 - <<'PY'
from pathlib import Path
workspace = Path("packages/toggl/endpoints/workspaces.ts").read_text()
binding = Path("packages/corsair/core/endpoints/bind.ts").read_text()
client = Path("packages/toggl/client.ts").read_text()
checks = {
"workspace get returns raw result": "return result;" in workspace[workspace.index("export const get:"):workspace.index("export const update:")],
"workspace get has no credential sanitizer": "withoutCredentials" not in workspace[workspace.index("export const get:"):workspace.index("export const update:")],
"request client returns parsed provider body": "return (await response.json()) as T;" in client or "return (await response.json())" in client,
"bound endpoint returns handler result without output parsing": "return res;" in binding and "endpointSchemas" not in binding,
}
for name, passed in checks.items():
print(f"{name}: {'PASS' if passed else 'FAIL'}")
if not all(checks.values()):
raise SystemExit(1)
PYRepository: corsairdev/corsair
Length of output: 2609
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- HTTP request implementation ---'
git ls-files packages/corsair/http | sed -n '1,160p'
rg -n -C 10 'export.*request|async function request|response\.json|return.*response|parse' packages/corsair/http --glob '*.{ts,tsx}' \
| sed -n '1,260p'
printf '%s\n' '--- direct endpoint exposure tests ---'
rg -n -C 8 'bindEndpointsRecursively|endpointSchemas|Workspaces\.get|workspaces\.get' packages/corsair packages/toggl --glob '*.{test,spec}.{ts,tsx}' \
| sed -n '1,260p'Repository: corsairdev/corsair
Length of output: 5079
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- locate corsair/http implementation ---'
git ls-files | rg '(^|/)(http|request)(/|\.|$)' | sed -n '1,240p'
rg -n -C 8 'export (async )?function request|function request\(|response\.json\(\)|ApiRequestOptions' . \
--glob '*.{ts,tsx}' \
--glob '!**/node_modules/**' \
--glob '!**/dist/**' \
--glob '!**/build/**' \
| sed -n '1,320p'Repository: corsairdev/corsair
Length of output: 18906
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- packages/corsair/http.ts ---'
cat -n packages/corsair/http.tsRepository: corsairdev/corsair
Length of output: 1458
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- shared request implementation ---'
cat -n packages/corsair/async-core/request.tsRepository: corsairdev/corsair
Length of output: 13797
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- workspace cache implementation ---'
sed -n '1,110p' packages/toggl/endpoints/persist.ts
printf '%s\n' '--- entity-store persistence behavior ---'
rg -n -C 8 'upsertByEntityId|class.*Entity|EntityStore|JSON\.stringify|workspace' packages/corsair packages/toggl --glob '*.{ts,tsx}' \
| rg -v 'endpoints\.test|integration\.test|schema\.test' \
| sed -n '1,320p'Repository: corsairdev/corsair
Length of output: 23384
Remove api_token from every workspace response.
The shared request client returns response.body unchanged, and endpoint binding does not apply TogglEndpointOutputSchemas. Apply one sanitizer to workspaces.get, workspaces.list, and workspaces.update. Add regression tests with api_token.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/toggl/endpoints/workspaces.ts` around lines 33 - 49, Remove
api_token from workspace response objects by introducing one shared sanitizer
and applying it in workspaces.get, workspaces.list, and workspaces.update before
returning results. Preserve all other workspace fields and add regression
coverage for responses containing api_token.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/toggl/endpoints/smail.ts`:
- Around line 68-72: Update the auditPayload call in the toggl.smail.sendMeet
completion logging to pass an empty identifier list instead of ['location'], so
the audit event records supplied field names without persisting the meeting
location.
In `@packages/toggl/endpoints/types.ts`:
- Around line 817-826: Update the operations schema to use a discriminated union
on op: keep remove as its own variant without value, and require value for add
and replace variants. Preserve the existing non-empty array constraint and path
validation.
- Around line 181-185: Update TogglQuotaSchema.organization_id to accept null
values by making its numeric schema nullable, while preserving the existing
required field and all other quota fields unchanged.
- Around line 697-729: Define and reuse a shared TogglIdSchema based on an
integer number, then replace every resource-ID z.number() input throughout the
types with it, including optional or nullable IDs and tag_ids/time_entry_ids
array elements. Update the affected schemas such as
OrganizationsGetGroupsInputSchema, OrganizationsCreateGroupInputSchema, and
OrganizationsDeleteGroupInputSchema while leaving non-ID numeric fields
unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c2b5d3f-344a-4ed5-9cd0-dcb2dadc8f2b
📒 Files selected for processing (17)
packages/toggl/client.test.tspackages/toggl/client.tspackages/toggl/endpoints-extended.test.tspackages/toggl/endpoints.test.tspackages/toggl/endpoints/index.tspackages/toggl/endpoints/me.tspackages/toggl/endpoints/organizations.tspackages/toggl/endpoints/projects.tspackages/toggl/endpoints/reference.tspackages/toggl/endpoints/smail.tspackages/toggl/endpoints/tasks.tspackages/toggl/endpoints/time-entries.tspackages/toggl/endpoints/types.tspackages/toggl/endpoints/webhook-subscriptions.tspackages/toggl/endpoints/workspaces.tspackages/toggl/index.tspackages/toggl/schema.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/toggl/endpoints.test.ts
- packages/toggl/schema.test.ts
- packages/toggl/client.ts
- packages/toggl/client.test.ts
|
@greptile review |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/toggl/endpoints/clients.ts`:
- Around line 119-125: Update the archive flow around the clientsArchive request
to define and use a response schema matching { items: number[] }, then remove
the cacheClient call because the archive response is not a TogglClient. If the
cache must reflect the archived state, fetch the client separately before
caching it with cacheClient.
- Line 58: Update the create payload in the relevant client creation method to
remove the wid field and input.workspace_id mapping. Preserve only the
documented body fields: external_reference, name, and notes.
In `@packages/toggl/endpoints/tasks.ts`:
- Around line 21-25: Branch the task query construction by route: when
project_id is set, send only the supported active parameter and omit page and
per_page; retain pagination parameters for the non-project task endpoint.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 236e6664-6647-4253-8e09-889ace078ce0
📒 Files selected for processing (12)
packages/toggl/client.test.tspackages/toggl/client.tspackages/toggl/endpoints-extended.test.tspackages/toggl/endpoints.test.tspackages/toggl/endpoints/clients.tspackages/toggl/endpoints/index.tspackages/toggl/endpoints/tags.tspackages/toggl/endpoints/tasks.tspackages/toggl/endpoints/types.tspackages/toggl/error-handlers.tspackages/toggl/index.tspackages/toggl/schema.test.ts
💤 Files with no reviewable changes (1)
- packages/toggl/client.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/toggl/endpoints/tags.ts
- packages/toggl/error-handlers.ts
- packages/toggl/endpoints/index.ts
- packages/toggl/client.test.ts
- packages/toggl/index.ts
- packages/toggl/endpoints-extended.test.ts
- packages/toggl/endpoints/types.ts
- packages/toggl/endpoints.test.ts
|
Remaining findings are being fixed by a bot commit — it will be re-reviewed automatically. |
|
@greptile review |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/toggl/endpoints/persist.ts (1)
1-35: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winKeep the best-effort cache contract and add rejection-path tests.
The cache is a non-authoritative mirror, so provider calls may succeed when
upsertByEntityIdordeleteByEntityIdfails. Add tests that assert failures are swallowed, warnings are emitted, and deleted records can remain stale.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/toggl/endpoints/persist.ts` around lines 1 - 35, Preserve the best-effort behavior implemented by safely: cache failures from upsertByEntityId and deleteByEntityId must be swallowed so provider calls still succeed, with a warning emitted for each rejection. Add rejection-path tests for both operations, including that failed deletion leaves the existing cached record stale.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/toggl/schema.test.ts`:
- Around line 250-258: Update the timeEntriesCreate.parse fixture in the
“rejects a non-RFC3339 time entry start” test to use a valid duration value,
keeping the intentionally invalid start unchanged so the assertion specifically
exercises timestamp validation.
---
Nitpick comments:
In `@packages/toggl/endpoints/persist.ts`:
- Around line 1-35: Preserve the best-effort behavior implemented by safely:
cache failures from upsertByEntityId and deleteByEntityId must be swallowed so
provider calls still succeed, with a warning emitted for each rejection. Add
rejection-path tests for both operations, including that failed deletion leaves
the existing cached record stale.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5fbf600c-50a6-4d58-b853-8f98853f372b
📒 Files selected for processing (20)
packages/toggl/client.test.tspackages/toggl/client.tspackages/toggl/endpoints-extended.test.tspackages/toggl/endpoints.test.tspackages/toggl/endpoints/clients.tspackages/toggl/endpoints/me.tspackages/toggl/endpoints/organizations.tspackages/toggl/endpoints/persist.tspackages/toggl/endpoints/projects.tspackages/toggl/endpoints/reference.tspackages/toggl/endpoints/smail.tspackages/toggl/endpoints/tags.tspackages/toggl/endpoints/tasks.tspackages/toggl/endpoints/time-entries.tspackages/toggl/endpoints/types.tspackages/toggl/endpoints/webhook-subscriptions.tspackages/toggl/endpoints/workspaces.tspackages/toggl/index.tspackages/toggl/integration.test.tspackages/toggl/schema.test.ts
🚧 Files skipped from review as they are similar to previous changes (18)
- packages/toggl/endpoints/projects.ts
- packages/toggl/client.test.ts
- packages/toggl/endpoints/smail.ts
- packages/toggl/endpoints.test.ts
- packages/toggl/endpoints/tasks.ts
- packages/toggl/endpoints/clients.ts
- packages/toggl/endpoints/me.ts
- packages/toggl/endpoints-extended.test.ts
- packages/toggl/integration.test.ts
- packages/toggl/client.ts
- packages/toggl/endpoints/tags.ts
- packages/toggl/endpoints/webhook-subscriptions.ts
- packages/toggl/endpoints/reference.ts
- packages/toggl/endpoints/workspaces.ts
- packages/toggl/index.ts
- packages/toggl/endpoints/time-entries.ts
- packages/toggl/endpoints/organizations.ts
- packages/toggl/endpoints/types.ts
|
@greptile review |
Maintainer review neededAutomated rounds are exhausted. Remaining findings:
Knowledge Base Used: The provider-plugin package pattern |
github-actions
Bot
added
the
needs-maintainer
# Conflicts: # packages/corsair/core/constants.ts
|
@greptile review |
|
Fixed a small bug |
|
LGTM tested locally API |
3 participants
Description
Adds a Toggl Track integration implemented against the Track API v9.
Fixes #671
73 operations across 11 resource groups, each with zod input and output
schemas, a declared risk level and a description. This covers the full 56-op
surface listed on corsair.dev/oss/toggl, plus a handful of natural companions
(update/delete on projects, tasks and time entries) that the catalog omits:
meworkspacesorganizationsclientsprojectstaskstagstimeEntriesreferencewebhookssmailAuth
Toggl uses HTTP Basic with the API token as the username and the literal string
api_tokenas the password. It is a per-user token with no OAuth flow and norefresh/expiry lifecycle, so it maps onto Corsair's
api_keyauth type directlyand
oauth_2is not offered.Rate limiting and errors
Toggl paces requests at roughly 1/sec per token per IP using a leaky bucket and
returns 429 on overflow, without a documented
Retry-After. The client isconfigured for 5 retries with 1s initial delay and 2x backoff, and every failure
routes through
error-handlers.ts(429, auth, permission, not-found, validation,network, default).
One quirk worth reviewer attention: Toggl answers a bad or revoked token with
403, not 401, so status alone cannot separate an auth failure from a permission
failure. The auth and permission handlers match on the response body to tell them
apart.
Schema design
Only the slow-changing structural records are persisted —
workspaces,clients,projects,tags. These resolve the ids nearly every other callneeds, change rarely, and caching them avoids spending the 1/sec budget on
lookups.
Time entries are deliberately not persisted: they are high-volume, they
mutate while a timer is running, and they are almost always wanted as a live view
rather than a stale local copy.
Scope notes
not. The catalog lists 0 triggers, so no handlers are registered, but the
subscription/status/event-filter operations are implemented against Toggl's
separate
/webhooks/api/v1host.GET /event_filtersconfirms Toggl emitscreated/updated/deleted for client, project and time_entry, so the payload
envelope and tenant matcher are left in place to make adding triggers additive.
smail.*) and the two email-unsubscribeoperations are implemented and covered by mocked tests, but deliberately never
fired against the live API — doing so would send real email.
and is out of scope here.
Checklist
Before submitting your PR, please verify the following:
pnpm lintand all checks passpnpm typecheckand there are no TypeScript errorspnpm buildand all packages build successfullypnpm testand all tests passScreenshots / Demos (if applicable)
120 tests total — 111 CI-safe + 9 live.
integration.test.tsruns against a real Toggl account and performs genuineround-trips: create/read/delete a client, create/list/delete a project,
create/rename/delete a tag, and start/stop/delete a running timer — each
validating the live response against the declared schema and cleaning up after
itself. Requests are paced at 1.1s for the leaky bucket.
The 111 CI-safe tests (
schema.test.ts,client.test.ts,endpoints.test.ts,endpoints-extended.test.ts)cover schema validation against captured real payloads, Basic auth header
construction, every error handler branch, and all 73 endpoint wrappers — their
paths, methods, query strings, request bodies, response normalisation and local
cache writes — with the network mocked.
Additional Notes
integration.test.tsis named to match the CI exclusion inpr-checks.yml, soit never runs without credentials. It also self-skips when
TOGGL_API_TOKENisabsent. Run it locally with:
TOGGL_API_TOKEN=<token> TOGGL_WORKSPACE_ID=<id> pnpm exec jest integrationdemo/testing/. CONTRIBUTING.md asks forthat, but R1 in PLUGIN_PR_RULES.md restricts a plugin PR to
packages/<plugin>/**, theconstants.tsregistration andpnpm-lock.yaml—so committing it would fail the scope gate. Live verification lives in
integration.test.tsinstead. Flagging in case the two documents should bereconciled.
getResponseBodyinpackages/corsair/async-core/request.tslogs a caughtSyntaxErrortoconsole.errorwhen a provider returns an empty body with aJSON content type. Toggl does this on every successful
DELETE, so the noiseshows up in test output even though the behaviour is correct.
created in the new Toggl app are not visible to the Track v9 API. A workspace
has to exist via
POST /api/v9/organizationsfor any workspace-scoped endpointto return data.
Summary by CodeRabbit