| title | Redundant API pre-checks in wrapper functions | ||||
|---|---|---|---|---|---|
| date | 2026-03-28 | ||||
| captured | 2026-03-28 05:38:33 UTC | ||||
| tags |
|
||||
| source | Claude Code session - github-mcp-worker project, reviewing push_skill tool | ||||
| aliases | |||||
| status | refined |
While reviewing the push_skill tool in the github-mcp-worker project (a Cloudflare Worker that commits files to GitHub via the Contents API), the code review revealed that every file push was making twice the expected number of API calls. The worker is rate-limited at 5,000 GitHub API requests/hour, so unnecessary calls matter, especially when pushing a skill with multiple extra files.
The push_skill tool calls a local getExistingFile() function to check if a file already exists (to detect "add vs update" for the commit message and to compare content for no-op detection). Then it calls createOrUpdateFile() from the shared GitHub library, which internally does the exact same GET request to fetch the file's SHA before writing.
push_skill flow (2 API calls per file, 1 redundant):
getExistingFile(path) → GET /contents/path (check #1: exists? content?)
createOrUpdateFile(path) → GET /contents/path (check #2: get SHA, redundant)
→ PUT /contents/path (actual write)
For a skill with SKILL.md + 3 extra files, that's 8 GETs where 4 would suffice.
This is a common pattern when a wrapper function pre-checks a condition that the library function already checks internally. It happens because:
- The library function (
createOrUpdateFile) was designed to be self-contained; it handles both create and update cases by checking for an existing SHA. - The wrapper (
push_skill) needs additional information from that same check (the file content, for diffing), so it does its own GET. - Nobody noticed the overlap because the two checks live in different files.
Three fix options:
-
Expose an optional SHA parameter on the library function. The wrapper passes the SHA it already fetched, and the library skips its internal check. Minimal API change, biggest efficiency win.
-
Split the library into check + write. Export
getFileSha()separately fromwriteFile(). The wrapper composes them explicitly. More flexible but more surface area. -
Accept the redundancy. If the rate limit headroom is large and the code clarity of self-contained functions matters more than 1 extra GET per file, do nothing. Valid choice for a single-user personal tool.
The right fix depends on scale. For a personal MCP worker doing < 50 pushes/day, option 3 is fine. For a shared service, option 1.
When a function calls an API to check state (exists? version? content?) and then passes the result to a library function that starts by checking the same state, you have a redundant pre-check. Look for this pattern whenever a wrapper function adds logic around a self-contained library call.
- [[why-knowledge-notes-need-context-not-just-facts]] - this note was one of the two original notes that revealed the "shallow capture" problem, leading to the context-first capture system
- [[youtube-transcript-extraction-from-cloud-containers]] - another example of debugging a multi-layer system where understanding which layer handles what prevents redundant work