Repository navigation
feat: cf setup interactive auth onboarding - #10
Merged
Merged
Conversation
jordanpartridge
force-pushed
the
cursor/cf-setup-auth-6412
branch
from
September 10, 2026 05:51
15ddf8d to
856471a
Compare
Contributor
Author
|
Added a happy-path test: mocked token verify succeeds, credentials are written to |
There was a problem hiding this comment.
Ship it.
- Non-interactive + JSON paths, token verification, and .env write all correct and tested.
- writeEnv handles replacement vs append and quoting; escape logic is sufficient for token/account values.
- No prod-breaking patterns; follows existing Saloon/LaravelZero conventions in the repo.
- UX note (non-blocking): interactive prompts only exercised manually; add a simple happy-path interactive test if desired later.
jordanpartridge
force-pushed
the
cursor/cf-setup-auth-6412
branch
from
September 10, 2026 13:35
c31d444 to
a134be3
Compare
There was a problem hiding this comment.
Blocking: writeEnv mutates .env without atomicity or backup (SetupCommand.php:95). A partial failure leaves a corrupt file.
- preg_replace_callback fallback can silently drop a key on regex error.
- CloudflareConnector constructor widened to take accountId even though VerifyToken ignores it.
Tests cover the happy and failure paths; CI gate still pending per spec. Fix the write safety issue before merge.
Add interactive and non-interactive setup that verifies the API token via Cloudflare's user/tokens/verify endpoint, writes credentials to .env, and documents ./cf setup as the primary configuration path. Co-authored-by: Jordan Partridge <jordan@partridge.rocks>
Co-authored-by: Jordan Partridge <jordan@partridge.rocks>
Failure paths were covered; this asserts token verify + .env write.
Cover missing token/account, verify fail, JSON fail, and happy-path .env write.
Write .env only after GET /user/tokens/verify succeeds. Non-interactive flags. Do not merge.
Replace existing keys with a callback so token values are not treated as preg backreferences. Verify still gates on envelope success only.
Also give tunnel:get OutputsJson so missing creds is JSON-safe and PHPStan is clean after the #8 merge.
jordanpartridge
marked this pull request as ready for review
September 12, 2026 15:38
jordanpartridge
force-pushed
the
cursor/cf-setup-auth-6412
branch
from
September 12, 2026 15:38
a134be3 to
c104a63
Compare
There was a problem hiding this comment.
Blocking: none. Stray edits and one spec/diff mismatch noted inline.
- TunnelGetCommand.php change (OutputsJson + null guard) is outside scope of cf-setup.
- .gitignore addition contradicts the locked spec claim that the entry already exists.
- writeEnv mutates .env without atomic write or backup; acceptable for onboarding but worth noting.
- Tests cover the documented non-interactive + JSON paths; interactive paths intentionally untested per spec.
Ship once the two inline items are addressed or explained.
There was a problem hiding this comment.
Ship it.
- SetupCommand correctly gates .env write behind successful VerifyToken and never leaks the token in JSON.
- Non-interactive + JSON paths tested; failure cases leave .env untouched.
- TunnelGetCommand change is a narrow compatibility patch, not a refactor.
- No contradictions with existing Saloon layout or command patterns.
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
Interactive and non-interactive
cf setupfor API token onboarding.setupcommand with--token,--account-id,--non-interactive, optional--json.env; README points here firstTest plan