Repository navigation
refactor: shared CloudflareCommand base and clearer errors - #6
Merged
Merged
Conversation
jordanpartridge
force-pushed
the
cursor/shared-command-base-6412
branch
from
September 10, 2026 05:53
ea8bbef to
07ee936
Compare
There was a problem hiding this comment.
Ship it.
- Removes all bare
exit(1)calls in favor of returning FAILURE. - New base class centralizes token handling, zone resolution, and error formatting.
- Test covers the missing-token case.
Three files still carry the now-unused InteractsWithCloudflare import after the inheritance change (TunnelCreate/Delete/List). Minor cleanup only.
Contributor
Author
|
Only |
Move resolveZoneId/truncate/formatApiError onto InteractsWithCloudflare, modernize dns:delete and dns:update onto the shared traits (no exit(1)), and use formatApiError across zone/tunnel failure paths. Co-authored-by: Jordan Partridge <jordan@partridge.rocks>
cursor
Bot
force-pushed
the
cursor/shared-command-base-6412
branch
from
September 13, 2026 00:46
07ee936 to
d581768
Compare
There was a problem hiding this comment.
Ship it.
- Base trait +
resolveZoneId/formatApiErrorcentralizes the duplicated logic that was scattered across every command. - All error paths now surface the Cloudflare
errors.0.messagewhen present; raw body only as fallback. getConnectornow returns nullable and the commands check it, removing the bareexit(1)calls.- Test expectations updated to match the new
jsonSuccess/jsonFailpayload shape. - No correctness or safety regressions observed.
jordanpartridge
marked this pull request as ready for review
September 13, 2026 00:47
There was a problem hiding this comment.
Ship it.
- Centralizes resolveZoneId, truncate, formatApiError in InteractsWithCloudflare; all prior duplicates removed.
- DNS delete/update now use traits + jsonFail instead of exit(1) and error().
- formatApiError applied to every failure path; JSON envelope aligned on tunnel:get.
- Tests updated for new payload shape.
No blocking issues. Minor duplication note between resolveZoneId and resolveZoneIdForHostname, but not a correctness problem.
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
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.
Summary
Rebased onto latest
main. Instead of a separate abstractCloudflareCommand(main already usesInteractsWithCloudflare+OutputsJson), this PR:resolveZoneId,truncate, andformatApiErrorhelpers toInteractsWithCloudflaredns:delete/dns:updateonto the shared traits (noexit(1))formatApiErroron zone/tunnel failure pathstunnel:getJSON with the agent envelopeTest plan
vendor/bin/pestlocallydns:delete/dns:update/tunnel:get --json