refactor: Dedup the tool-loading mode-resolution gate#1078
Draft
jirispilka wants to merge 1 commit into
Draft
Conversation
What we're solving: loadToolsByName, loadToolsFromUrl, and loadToolsFromInput each repeated the same buffer-or-compose block after getActors() — buffer the fetched actor tools when the server mode isn't resolved yet, otherwise compose the mode-specific set and upsert. The three differed only in the `notify` value passed to upsertTools (and loadToolsFromUrl's debug logs), and the reason those notify values differ was documented only on updateToolsAfterServerModeResolved. How: extract the shared block into a private registerFetchedActorTools(input, actorTools, notify) helper. Each loader keeps its own pre-work and getActors call and passes its notify value: loadToolsByName = actorTools.length > 0 (its pre-mode upsert only ran under that guard, so the old literal `true` is equivalent); loadToolsFromUrl and loadToolsFromInput = false. The helper carries a doc comment explaining the intentional notify divergence and points to updateToolsAfterServerModeResolved for the full rationale. loadToolsFromUrl's two per-branch debug logs collapse into one line emitted before the helper call. Adds a regression test asserting the per-loader notify behavior (loadToolsByName notifies eagerly pre-init; the other two defer to the post-initialize reconcile), which was previously untested. Alternatives considered: folding getActors into the helper (rejected — loadToolsByName pre-filters missing names before fetching); a debugMessages param to preserve both fromUrl log lines verbatim (rejected — over-engineered for two debug breadcrumbs; a single log line is clearer). Note: the notify flag gates notifyToolsChangedHandler, which the hosted server uses for Redis-backed state recovery. Behavior is byte-identical (same notify values and timing); the only change is the debug-log consolidation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016pXZvhEoE6Lz3DpBSL3n1Y
9 tasks
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
3 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.
Part of #1066 (checkbox: dedup the mode-resolution gate).
What we're solving
loadToolsByName,loadToolsFromUrl, andloadToolsFromInputeach repeated the same buffer-or-compose block aftergetActors()— buffer the fetched actor tools when the server mode isn't resolved yet, otherwise compose the mode-specific set and upsert. The three differed only in thenotifyvalue passed toupsertTools(and loadToolsFromUrl's debug logs), and the reason those notify values differ was documented only onupdateToolsAfterServerModeResolved.How
Extract the shared block into a private
registerFetchedActorTools(input, actorTools, notify)helper. Each loader keeps its own pre-work andgetActorscall and passes its notify value:loadToolsByName=actorTools.length > 0— its pre-mode upsert only ran under that guard, so the old literaltrueis equivalent.loadToolsFromUrl/loadToolsFromInput=false.The helper carries a doc comment explaining the intentional notify divergence and points to
updateToolsAfterServerModeResolvedfor the full rationale (that comment is retained).loadToolsFromUrl's two per-branch debug logs collapse into one line emitted before the helper call.Adds a regression test asserting the per-loader notify behavior (
loadToolsByNamenotifies eagerly pre-init; the other two defer to the post-initialize reconcile) — previously untested.Alternatives considered
getActorsinto the helper — rejected;loadToolsByNamepre-filters missing names before fetching.debugMessagesparam to preserve both fromUrl log lines verbatim — rejected; over-engineered for two debug breadcrumbs, a single line is clearer.The
notifyflag gatesnotifyToolsChangedHandler, which the hosted server (apify-mcp-server-internal) uses for Redis-backed state recovery. Behavior is byte-identical — same notify values and timing — verified against the pre-change code. The only observable change is the debug-log consolidation (twolog.debuglines → one; debug-only).Verification
test:unit963 passed / 2 skipped (+4 new notify regression cases), format no-op.pendingToolsAfterModeResolved.pushnow appears once (in the helper).src/mcp/server.ts+tests/unit/mcp.server.capability_gating.test.ts.🤖 Generated with Claude Code
https://claude.ai/code/session_016pXZvhEoE6Lz3DpBSL3n1Y
Generated by Claude Code