Repository navigation
MCP: Update Node SDK to 0.132.0 - #258
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
299c7d1 to
aed5e24
Compare
meliaj
left a comment
There was a problem hiding this comment.
reviewed the sdk 0.131.0 parity changes. three things worth a look, the first two are the ones i'd fix before merging:
Bugs
-
src/lib/mcp/tools/api-keys.ts:211-222—rotateacceptsexpires_atandpolicybut never forwards them, and returns success.- repro:
rotate {api_key_id: "k1", expires_at: "2027-01-01T00:00:00Z", policy: {...}}returns a new plaintext key, and the sdk is called asrotate("k1", {}). - the sdk's rotate only takes
days_to_expireandexpire_in_days, so the replacement key silently reuses the old key's lifetime (or never expires), andpolicyis dropped. - fix: return an error when
expires_atorpolicyis set on any action other thancreate, and whenexpire_in_daysis set on any action other thanrotate. a test for each would cover it.
- repro:
-
src/lib/mcp/tools/browser-pools.ts:136—regionis only read in the create builder, andtagsis only read in the acquire path, so both are silently ignored on the other actions.- repro:
update {id_or_name: "p", region: "us-west", size: 5}succeeds and the sdk gets{size: 5}.updatewithregionalone fails with "at least one update field is required", which hides the real cause.tagsis dropped the same way on create and update. - fix: reject
regionon update andtagson create/update the same wayexpected_networkandsave_profile_changesalready are inbuildPoolCreateParams/buildPoolUpdateParams, with tests. - nit:
memory,video_memoryandregioninmanage_browsershave the same problem on update, so worth fixing in the same pass.
- repro:
Compatibility
src/lib/mcp/vault-responses.ts:257— the newfield !== nullcheck changes the output for existing fields, not justagent_wallet_step_up.action_url.- repro: projecting
{type: "card", action: {name: "x", url: null}}givesaction: {name: "x"}onmainandaction: {name: "x", url: null}on this branch. - any item response with a null url-class field now has
"key": nullwhere the key used to be omitted. i didn't find a test that pins the old shape. - consider scoping the null passthrough to the new
action_urlfield, or adding a test that documents the shape change if it's intended.
- repro: projecting
aed5e24 to
a8c8036
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a8c8036. Configure here.
- manage_api_keys: expires_at and policy are create-only; expire_in_days is rotate-only. - manage_browser_pools: tags are acquire-only; region cannot change on update. - manage_browsers: memory, video_memory, and region are creation-only. - Vault responses pass a null URL through only for agent_wallet_step_up.action_url, so other null URL fields stay omitted as before.
|
@meliaj thanks. All three are addressed in the latest commit on this branch:
The 12 new tests fail without the fixes. |
There was a problem hiding this comment.
approving at e975199. it fixes the three issues from my earlier review.
fixed
manage_api_keys:expires_atandpolicyare rejected unless the action iscreate, andexpire_in_daysunless it isrotate.rotateno longer returns a key with a different lifetime than the caller asked for.manage_browser_pools:tagsis rejected on create and update, andregionon update. a loneregionon update now gets a clear error instead of "at least one update field is required".manage_browsers:memory,video_memoryandregionare rejected onupdate.- vault responses: a null URL is only passed through for
action_url, so other null URL fields are omitted as before.
how i tested
- read the commit diff and its tests. each new test asserts the SDK is not called.
bunx tsc --noEmitis clean andbun test src/lib/mcppasses (995 tests, 0 failures). i did not run the fullbun testor read the CI logs.- ran probes against the real code on this head: the link card schema accepts
amount: 50000and rejects 50001 and 500000, and the vault projection drops unknown keys and an unsafeaction_urlwhile keeping the allowlisted fields. - checked the passkey result schema and the link user-info allowlist against the 0.132.0 SDK types. both match.
scope
this approval covers the fix commit and the earlier findings only. i did not re-review the rest of the diff against 0.132.0.
non-blocking follow-ups
docs/vault-payments.md:595still describes a separate pay-token amount limit, but the schema now caps all link card amounts at 50000. worth rewording, and worth a changelog note since 50001–500000 is now rejected.- smaller gaps of the same kind:
days_to_expireandproject_idon non-create api-key actions,tagsandregionon other pool actions, andgpuon browserupdateare still silently ignored. - the passkey parse-failure message could add "if this was arm_passkey, invoke disarm_passkey".
- manage_apps: app_name, version, query, action_name, deployment_id, invocation_id, and since must be nonempty, so an empty list_invocations filter no longer narrows results to nothing. - manage_browser_pools: id_or_name and name must be nonempty. - manage_api_keys: api_key_id must be nonempty, and days_to_expire is rejected on actions other than create and rotate. - manage_proxies: the deprecated proxy_id alias must be nonempty, and missing-selector errors name id_or_name.

Summary
@onkernel/sdkto0.132.0.Coverage
MCP coverage report
Added or updated coverage
APIKey.last_used_at; it contains no method, request-parameter, nested-option, enum, pagination, streaming, or signature changes.manage_api_keysactionslistandgetalready return SDK API-key objects without field projection, solast_used_atis now exposed automatically throughAPIKeys.listandAPIKeys.retrieveresponses.nullsurvive list pagination and get response handling in compact JSON.Description updates
manage_api_keystool description for OpenAPI schema fieldApiKey.last_used_at: list/get responses carry an approximate last-authentication timestamp accurate within about 20 minutes, ornullwhen the key has not authenticated a request since last-use tracking began.Intentional exclusions
The prior deliberate exclusions remain unchanged because SDK 0.132.0 adds no methods or request parameters:
Validation
bun run check:managed-auth-app: passed.bunx tsc --noEmit --incremental false: passed.bun test: passed — 1110 tests, 0 failures, 4921 expectations across 77 files.git diff --checkis clean.origin/main. No production API calls or real resources were used.Validation
Note
Medium Risk
Broad exposure of security-sensitive controls (API key policy/rotate, vault passkeys, network allowlist mutations) with validation layers, but behavior delegates to the Kernel SDK rather than new server-side auth logic.
Overview
Bumps
@onkernel/sdkfrom 0.126.0 to 0.132.0 and expands the MCP tool layer so agents can use the newer API surface with stricter Zod validation and action-scoped parameter guards.API keys gain
rotate, policy-scoped create, expiry options, richer list/get filters, and documentedlast_used_aton responses. Browsers and pools add memory/region (includingus-west), multi-extensionextensionsarrays, fulltelemetry(plusplatformin the catalog), location overrides, get_telemetryevent_types, and conditional allowlist updates. Apps add list_invocations, delete_invocation_browsers, async invoke timeout, and location on invocation browsers. Managed auth adds login-onlyskill_modeand expanded browser telemetry (storage). Smaller parity updates cover proxies (id_or_name), extensions get, list query/name filters across resources, exec_commandenv/as_user, vault passkey / add_fields / remove_fields, and rejecting deprecatedmax_pooled_sessions.Several tools adopt injectable
McpDependenciesfor tests; coverage is expanded with SDK-parity unit tests across the touched tools.Reviewed by Cursor Bugbot for commit 3c43e48. Bugbot is set up for automated code reviews on this repo. Configure here.