feat: add time entry CRUD tools (closes #7, revisits #8 and #12) - #38
Open
madisonrickert wants to merge 3 commits into
Open
feat: add time entry CRUD tools (closes #7, revisits #8 and #12)#38madisonrickert wants to merge 3 commits into
madisonrickert wants to merge 3 commits into
Conversation
Toggl returns HTTP 200 with content-length: 0 (not 204) on some write
endpoints, including DELETE /workspaces/{wid}/tags/{tid} and
DELETE /workspaces/{wid}/time_entries/{tid}, per the official Toggl
Engineering API docs. The existing request() short-circuited only on
204, so it called response.json() on the empty body, which threw and
triggered the retry loop without noRetry. The retried call then got a
real 404 because the first call had already succeeded server-side,
surfacing the misleading "Tag was not found" / "not found" errors to
callers despite the operation having completed.
Treat 200 with content-length: 0 (or empty body when Content-Length is
absent) the same as 204. Behavior unchanged for non-empty success
responses.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
madisonrickert
force-pushed
the
feat/time-entry-crud
branch
from
May 2, 2026 19:02
eb9ea11 to
0775b05
Compare
Add three MCP tools that complete the time entry CRUD surface, plus a
small extension to start_timer:
toggl_create_time_entry POST /workspaces/{wid}/time_entries
toggl_update_time_entry PUT /workspaces/{wid}/time_entries/{tid}
toggl_delete_time_entry DELETE /workspaces/{wid}/time_entries/{tid}
toggl_start_timer + billable param
The TogglAPI methods already existed; this wires them as MCP tools using
the post-1.1.0 patterns: resolveWorkspaceForTool for workspace
resolution, jsonResponse for output, hydration via cache.hydrateTimeEntries
on returned entries, and structured Error throws caught by the central
errorPayload handler. Update tool only sends fields the caller passed,
so callers can pass {project_id} alone without clobbering description.
Common workflow this enables: an AI agent reviews uncategorized entries
returned by toggl_get_time_entries and assigns project_id/task_id via
toggl_update_time_entry — the use case from verygoodplugins#7.
Cache: time entries aren't cached as a collection, so no invalidation
needed. Hydration via cache.hydrateTimeEntries handles project/client
name resolution on responses.
Closes verygoodplugins#7. Resolves the work attempted in closed PRs verygoodplugins#8 (rocketworm) and
verygoodplugins#12 (arndjan), reimplemented from scratch on current main per the
maintainer's "fresh PR from current main with tests against the
post-1.1.0 handler structure" guidance.
Co-Authored-By: Andreas Wurm <mail@andreaswurm.at>
Co-Authored-By: Arnd Jan Gulmans <arndjan@gmail.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Some Toggl plans (Free) silently treat all time entries as non-billable even when the project itself is billable. The flag is accepted by the API and project_billable still reflects the project setting, but the per-entry billable returns false on these plans. Annotate the billable parameter on toggl_start_timer, toggl_create_time_entry, and toggl_update_time_entry so agents see the limitation in the tool schema and don't waste turns assuming the flag applied or treating the discrepancy as broken behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
madisonrickert
force-pushed
the
feat/time-entry-crud
branch
from
May 2, 2026 19:10
0775b05 to
4a5abe3
Compare
madisonrickert
marked this pull request as ready for review
May 2, 2026 19:14
This was referenced May 2, 2026
Member
|
I opened a maintainer refresh branch as #58: #58 This keeps the time-entry CRUD work moving without force-pushing to the contributor fork, and stacks the implementation on the refreshed self-host HTTP foundation from #36. The refresh preserves the CRUD intent here, adds the missing read-only toggl_list_tasks lookup requested in #7, adds MCP handler coverage, disables retries for ambiguous write failures, handles Toggl 200/empty-body write responses, and validates write-tool arguments before calling Toggl. Original contributor attribution is included in the commit and PR body. Local verification on #58:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds full time entry CRUD per #7, reimplemented from current
mainper the maintainer's "fresh PR from current main with tests against the post-1.1.0 handler structure" note when closing #12 (and equivalent guidance closing #8).Note
Tracking #36, depends on #37. Same coordination story as #37 (tag CRUD): authored against current
main. The request 200/empty-body fix is cherry-picked from #37 because tag DELETE and time entry DELETE share the same Toggl response shape — patch-id collision will let the duplicate commit drop cleanly when either PR rebases. Happy to rebase against post-#36/#37mainand add handler tests using the newtests/mcp-server.test.tspattern, or merge in any order — your call.What's added
Three MCP tools that complete the time entry CRUD surface, plus a small extension to
toggl_start_timer:toggl_create_time_entry—POST /workspaces/{wid}/time_entries. Required:start. Optional:stop,duration,description,project_id,task_id,tags,tag_ids,billable. Fills the create gap for retroactively logging past work —toggl_start_timeronly creates running timers.toggl_update_time_entry—PUT /workspaces/{wid}/time_entries/{tid}. All fields optional exceptentry_id; only fields the caller passed are sent, so{ project_id: 50 }doesn't clobber description.toggl_delete_time_entry—DELETE /workspaces/{wid}/time_entries/{tid}.toggl_start_timer— gains optionalbillableparameter for parity.All three new tools resolve workspace via
resolveWorkspaceForTool, return viajsonResponse, and hydrate response entries viacache.hydrateTimeEntries. Errors flow through the central catch witherrorPayload(error). No new cache invalidation — time entries aren't cached as a collection.Includes a small
request()fix (cherry-picked from #37)toggl_delete_time_entryhits the same Toggl 200/content-length: 0shape as tag DELETE. The existingrequest()short-circuited only on 204 and calledresponse.json()on the empty body, throwing and triggering a misleading retry that surfaced as a bogus 404. The cherry-picked fix from #37 treats 200/empty-body the same as 204. Behavior unchanged for non-empty success responses.Resolves prior closed work
toggl_update_time_entrywith project/task/description/tags/billable. Implemented.start_timer+ update_entry tool. Maintainer's closing note: "rewrite from current main if billable/update-entry support is still wanted." Reimplemented from currentmain; original author credited viaCo-Authored-By.main; original author credited viaCo-Authored-By.Both contributors' branches were too far behind to cherry-pick cleanly (pre-1.1.0). Their work informed the design but the implementation here is fresh against current
mainpatterns:WorkspaceResolutionError,jsonResponse,errorPayload,resolveWorkspaceForTool,cache.hydrateTimeEntries.Plan-tier billable note
A small UX hint: the
billableparameter description on all three tools that accept it now mentions that some Toggl plans (Free) silently set entry-level billable tofalseeven when the project itself is billable. Agents can check the response to detect the discrepancy rather than assuming the flag applied. Verified live during smoke testing — the Toggl API accepts the flag and returns 200, but the response showsbillable: falseon Free workspaces.Commits
Three commits:
fix(api): handle Toggl 200/empty-body responses— cherry-picked from feat: add tag CRUD tools (revisits #9) #37feat: add time entry CRUD tools and billable on start_timer— Co-Authored-By: Andreas Wurm, Arnd Jan Gulmansdocs: note plan-tier billable behavior in tool schemasVerification
npm test— 36/36 passing (5 new API-level tests: POST/PUT/DELETE shape, billable-passthrough, 4xx no-retry)npm run build— cleannpm run lint— no new warningscreatewith billable →list(verify visible) →update(assign project, hydrate via cache) →delete→ verify cleanup. The empty-body fix is verified live; delete returns cleansuccess: true, no false 404.Coverage with #36's proposed thresholds
Same approach as #37 — every metric improves over
main; further gain available via handler tests oncecreateTogglServerlands.Scope
Time entry CRUD only. No project/client write tools, no SDK upgrade.