feat: add tag CRUD tools (revisits #9) - #37
Open
madisonrickert wants to merge 4 commits into
Open
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>
Add createTag/updateTag/deleteTag against the documented Toggl v9
endpoints:
POST /workspaces/{wid}/tags
PUT /workspaces/{wid}/tags/{tid}
DELETE /workspaces/{wid}/tags/{tid}
deleteTag returns void: Toggl emits HTTP 200 with empty body, which
the upstream request() change in the prior commit now treats as
successful. Tests pin endpoint/method/body shape per verb and the
4xx-no-retry contract for write operations.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add invalidateWorkspaceTags(workspaceId) to CacheManager. Drops the workspace tag-list entry and any single-tag entries belonging to that workspace, so the next read refetches from Toggl. Single-workspace scoping preserves cached state for unrelated workspaces. Tag CRUD tools call this after each successful create/update/delete to keep listings consistent with Toggl. Invalidation runs only on success: if the API rejects a write, server state is unchanged and the cache is still accurate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add four tools backed by the Toggl tag CRUD methods and workspace tag cache invalidation: toggl_list_tags (read; cache-backed) toggl_create_tag (write; idempotentHint: false) toggl_update_tag (write; idempotentHint: true; rename in place) toggl_delete_tag (write; destructiveHint: true) All four resolve workspace_id via resolveWorkspaceForTool, returning WorkspaceResolutionError when ambiguous. Write tools invalidate the workspace tag cache only after a successful API call. Runtime input checks guard against type-coerced or whitespace-only inputs that the JSON schema does not catch at the tool boundary. README adds a Tag Management section and a list-tags row under Lookups. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
madisonrickert
marked this pull request as ready for review
May 2, 2026 18:07
This was referenced May 2, 2026
Member
|
Thanks for the original tag CRUD work here. I created a maintainer refresh branch/PR at #59 with attribution preserved. What changed in the refresh:
I’m leaving this PR open for traceability while #59 moves through review. |
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
2 participants
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.
Reintroduces tag CRUD per the closing note on #9, with the requested tests and cache invalidation against the current bounded-bucket
CacheManager.Note
Tracking #36. PR was authored against current
main(v1.1.0). I see #36 introduces acreateTogglServer()factory and a 50% Vitest coverage gate onsrc/**/*.ts. Happy to rebase onto post-#36mainand add handler tests using the newtests/mcp-server.test.tspattern if that's useful — or merge in either order, your call. Local cherry-pick ontofeat/self-host-streamable-httpwas clean for commits 1–3; commit 4's conflict is mechanical (porting tool defs + cases into the new factory structure).What's added
Four MCP tools:
toggl_list_tags— read; cache-backedtoggl_create_tag— write;idempotentHint: falsetoggl_update_tag— write; rename in place;idempotentHint: truetoggl_delete_tag— write;destructiveHint: trueEach write tool calls
cache.invalidateWorkspaceTags(workspaceId)after a successful API call. The invalidation is workspace-scoped: it dropstagsByWorkspace[wid]and any single-tag entries with thatworkspace_id, leaving unrelated workspaces' caches untouched. Invalidation runs only on success, so cache state stays consistent with Toggl when a write is rejected.All four tools resolve workspace via the existing
resolveWorkspaceForToolhelper, returning structuredWorkspaceResolutionErrorpayloads when ambiguous.Includes a small
request()fixTag DELETE returns HTTP 200 with
content-length: 0per the Toggl API docs. The existingrequest()short-circuited only on 204, so it calledresponse.json()on the empty body, threw, retried (nonoRetryflag on the JSON parse error), and the second attempt got a real 404 because the first call had already succeeded server-side — surfacing as a misleading"Tag was not found"error to callers despite Toggl having completed the deletion.Fix: treat HTTP 200 with
content-length: 0(or empty body when Content-Length is absent) the same as 204. Behavior unchanged for non-empty success responses on every other endpoint. As a side benefit the same fix coversdeleteTimeEntry, which has the same Toggl response shape.Happy to split this fix into a separate PR if preferred — it stands alone, but tag DELETE depends on it being in place.
Commits
Four logical commits, each independently building and passing tests:
fix(api): handle Toggl 200/empty-body responsesfeat(api): add tag CRUD methods to TogglAPI clientfeat(cache): add workspace tag invalidationfeat(tools): expose tag CRUD as MCP toolsVerification
npm test— 36/36 passing (5 new tests: API verb shape per CRUD method, 4xx no-retry contract, 200/empty-body regression, cache invalidation refetch + workspace-isolation)npm run build— cleannpm run lint— no new warningsCoverage with #36's proposed thresholds
I ran
npm run test:coverageagainst the proposedvitest.config.tsfrom #36 (50% threshold across all metrics,include: ['src/**/*.ts']). Numbers, this PR vs currentmain:Every metric improves; functions cross the threshold.
Scope
Tag CRUD only. No project/client/time-entry write tools, no SDK upgrade, no opportunistic refactoring.