Uh oh!
There was an error while loading. Please reload this page.
fix(tailscale): align tool coverage and outputs with the Tailscale API - #5366
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview API alignment fixes include mapping The Tailscale block gains wandConfig on DNS nameserver and search-path fields, plus updated templates/skills for ACL drift remediation, full offboarding, policy-as-code ACL updates, and compromised-device lockdown. Reviewed by Cursor Bugbot for commit 7f43c6e. Configure here. |
Greptile SummaryThis PR fixes API alignment bugs and adds four new Tailscale tools (
Confidence Score: 3/5The new tools and bug-fixes are well-implemented, but two unaddressed changes alter the observable behaviour of existing tools in ways not mentioned in the PR description. The four new tools are clean and the field-alignment fixes are correct. However, two existing-tool changes introduced in this PR carry real downstream risk: adding ?all=true to list_auth_keys silently returns revoked and expired keys to every existing workflow that calls that tool (previously only active keys were returned), and removing magicDNS from list_dns_nameservers output contract breaks any workflow that reads output.magicDNS. Both were raised in the previous review round and have not been addressed. apps/sim/tools/tailscale/list_auth_keys.ts and apps/sim/tools/tailscale/list_dns_nameservers.ts Important Files Changed
Reviews (5): Last reviewed commit: "fix(tailscale): list_auth_keys now retur..." | Re-trigger Greptile |
Uh oh!
There was an error while loading. Please reload this page.
waleedlatif1
commented
Jul 2, 2026
Fixed — |
waleedlatif1
commented
Jul 2, 2026
waleedlatif1
commented
Jul 2, 2026
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit fdf867f. Configure here.
waleedlatif1
commented
Jul 2, 2026
waleedlatif1
commented
Jul 2, 2026
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6fd8469. Configure here.
- fix list_users profilePicUrl field name (API returns lowercase, was always null) - add nodeId, keyExpiryDisabled, expires to device outputs - quote the If-Match header value on ACL updates per API spec - add set_acl, expire_device_key, suspend_user, delete_user tools - add wandConfig to dnsServers/searchPaths block fields - expand BlockMeta skills/templates for ACL and key-expiry workflows
The GET /tailnet/{tailnet}/dns/nameservers response only returns
{dns: string[]} per the API spec — magicDNS is not part of this
endpoint's response and was always silently false.Matches the pattern used by the other new tools in this PR (delete_user, suspend_user).
GET /tailnet/{tailnet}/keys silently scopes to the caller's own
keys unless all=true is passed, contradicting the tool's stated
purpose of listing all auth keys in the tailnet.6fd8469 to
7f43c6eCompareUh oh!
There was an error while loading. Please reload this page.
waleedlatif1
commented
Jul 2, 2026
waleedlatif1
commented
Jul 2, 2026
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 7f43c6e. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
waleedlatif1
commented
Jul 2, 2026
Good catch — fixed. Restored |
waleedlatif1
commented
Jul 2, 2026
waleedlatif1
commented
Jul 2, 2026
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7f43c6e. Configure here.
| success: true, | ||
| output: { | ||
| dns: data.dns ?? [], | ||
| magicDNS: data.magicDNS ?? false, |
There was a problem hiding this comment.
List DNS drops magicDNS output
Medium Severity
The list_dns_nameservers tool no longer returns magicDNS, but the Tailscale block still exposes magicDNS as a block output and the PR stated no backwards-incompatible field removals. Workflows that read magicDNS after List DNS Nameservers now get a missing value instead of the prior always-false field.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7f43c6e. Configure here.


Summary
list_usersreadingprofilePicURLinstead of the API's actualprofilePicUrlfield — was always returning nullnodeId(API's preferred device identifier),keyExpiryDisabled, andexpiresto device outputsset_aclIf-Matchheader to be quoted per the Tailscale API specset_acl(write policy back, not just read),expire_device_key,suspend_user,delete_user(proper offboarding — the existing flow only deauthorized devices, never touched the user account)wandConfigto thednsServers/searchPathsblock fields per repo conventionTailscaleBlockMetaskills/templates for ACL management and key-expiry incident responseValidated every one of the 24 tools' endpoints, methods, required/optional params, and response field extraction against the live Tailscale OpenAPI spec and Go SDK source. Two independent passes confirmed full API alignment and that no backwards-incompatible field removals were made.
Type of Change
Testing
Tested manually. Verified against live Tailscale OpenAPI spec + Go client source.
bun run lintandbun run type-checkclean.Checklist