fix(cloud): single-select provider picker and a read-only Cloudflare step - #47
Conversation
…step From the 2026-08-19 demo feedback on onboarding. Three things in `cloud connect` were confusing: - `--multi` handed the user a multi-select where space toggles and enter confirms, which reads as a dead enter key. Removed, along with the repo-marker pre-selection it existed to carry: the picker is a plain single select again, and the installer's "Connect another cloud account?" loop is what covers a multi-cloud stack. - The Cloudflare instructions spent four sentences on permissions the user is told not to touch, and offered a choice between two docs links (#46). They now say the one thing that matters: the link opens Cloudflare's token screen with a pre-filled, read-only token, create it as-is. - "Does the token have write permissions?" is obsolete — the docs page offers one link and it mints a read-only token, which is what the console already sends. The question and the `--read-only` flag are gone; the CLI always connects Cloudflare read-only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
434bde3 to
06a3073
Compare
There was a problem hiding this comment.
The structural side of this is in good shape: the branch is a clean rebase onto current main (merge-base is exactly v0.2.15's 1fc91a9), the --multi removal was redone against the post-#45 ConnectOutcome shape (tally, --reconnect, and timeout exit codes all intact), the old "--multi tallies" comment got reworded, and the companion reference points at polylanedotcom#128. CI is 3/3 green at 06a3073, and locally at that head: typecheck clean, lint clean, 272/272 tests, build + --version smoke pass. Installer compat verified by execution: the old installer's probe (cloud connect --multi --help) exits 2 against this build and falls through to the picker loop as designed; the new installer's loop (polylanedotcom#128) works against either CLI.
One thing blocks, and it's cheap to fix — the Cloudflare copy vs. the page it links (connect.ts:345-346, :355):
The new instruction says "The link opens Cloudflare's account API token screen with a pre-filled, read-only token — create it as-is", but the link is the docs page, and nominal main renders TWO buttons there — "Create read and write token" first ("lets agents fix issues directly — deploys, config, rollbacks"), read-only second. That two-button page is nominal#1265, requested and merged by you at 15:50Z today, and cli#46's copy was written to describe it (same Slack thread, F11) — so this PR's replacement sentence is the side that now disagrees with the docs page. The concrete failure: a user follows the first button, mints a write-capable token, pastes it "as-is", and connect.ts:355 stamps it readOnly: true — a write-capable secret stored under a read-only label, and nominal's reviewApiCall then refuses every write for the account, silently disabling the capability the docs page advertises.
The always-read-only default itself is sound and matches both console connect surfaces (CloudflareConnectSlideover.vue / CloudflareForm.vue hardcode readOnly: true). Three coherent ways out — happy with any, but it should be one of them explicitly:
- Point the
link:at the read-only pre-filled dashboard URL directly (what the console token modal does viabuildCloudflareTokenUrl({ readOnly: true })) — one line, makes "create it as-is" true, no cross-repo coordination. - Keep the docs-page link but say "use the read-only link on the docs page".
- Keep this copy AND ship a nominal docs change collapsing the page to a single read-only link — a real product decision (it reverses #1265's design from this afternoon) that should be made on purpose, not via conflict resolution.
If your explicit call is that read-only-only is the product posture and the docs page will follow, say so — this finding then downgrades to a coordination note and the verdict flips on re-review.
Non-blocking notes:
--read-onlynow hard-errors (Unknown flag, exit 2, verified) — anyone who scripted it was asking for what's now the default and gets an error instead of a no-op; consider a one-release accept-and-ignore or a release note.- Scripted
--provider cloudflare --token Xconnects (no flag) silently flip from server-default write-enabled (zConnectCloudflareAccountdefaultsreadOnlyto false) to read-only — worth a release-note line too. - PR body says "npm test 258/258"; the rebased head's suite is 272/272.
- The dropped "starts with cfat_ / shown only once" facts are fine to leave to the docs page if that's the intent.
Generated by Claude Code
…o-op Review catch on the Cloudflare step: the CLI links to the docs page, and that page (nominal#1265, merged today) renders two pre-filled token links with "Create read and write token" first. Telling the user the link mints a read-only token and to paste it "as-is" would walk them into minting a write-capable token that this command then stores under readOnly: true — nominal's reviewApiCall refuses every write for such an account, so the capability the docs page advertises would be silently off with no CLI path to re-enable it. The instruction now names the "Create read-only token" link by its button label, so following it literally produces the token the command actually claims to store. Read-only stays unconditional: it is the demo-feedback posture and what both console connect surfaces send. The docs page leading with the write token is the other half of that decision and is called out on the PR for a human to settle. Also restores --read-only as an accepted no-op for one release: everyone who passed it was asking for what is now the default, and dropping it outright turned those invocations into exit 2 on an unknown flag. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Blocking finding was right, and my grounding was wrong: I checked What I did, and why not option 1. Pointing On the posture. Read-only stays unconditional. That is the explicit demo feedback ("remove the write-permissions question and all write-functionality options"), and it matches both console connect surfaces. But you are right that this leaves the CLI and the docs page stating different things — the docs page says Polylane "requests operational (edit) access so agents can act on your infrastructure". I have not resolved that here, because collapsing the docs page to read-only would reverse #1265 on the same day it merged. It is called out in the PR body and in the review thread in #pr-reviews for a human to settle: either the docs page follows the CLI, or the CLI needs a way back to a write token. Non-blocking notes, all addressed:
Re-requesting review. |
There was a problem hiding this comment.
LGTM: the blocking finding from the last round is resolved — the Cloudflare instruction now names the docs page's "Create read-only token" link character-for-character (verified against nominal main's connectGuides.ts), so following it literally produces exactly the token the command stores, and --read-only is back as a verified no-op instead of an exit-2 break.
Verified at bdc4c9b: CI 3/3 green (Node 20/22/24); locally typecheck, lint, and build + --version smoke all clean, and the suite is green — my full-suite runs reported 267 because the known pre-existing output.test.ts flake (its stdout capture can swallow the 5 formatValue results from the TAP count) hit both times; those 5 pass 5/5 in isolation and the file is untouched by this PR, so all 272 pass. Execution checks: cloud connect --read-only parses and is inert (nothing reads the flag; it now reaches the auth gate instead of dying in the parser, and the deprecation line shows in --help), while --multi and the old installer probe (--multi --help) still exit 2, so the pre-coreplanelabs/polylanedotcom#128 installer falls through to the picker loop correctly. The always-read-only posture checks out on the product side too: both console connect surfaces hardcode readOnly: true, and the console's own Cloudflare docs panel (clouds-cloudflare.ts) already describes a single read-only token link.
Deferring the docs-page divergence rather than resolving it silently is the right call, in my view: docs.polylane.com/integrations/cloudflare (coreplanelabs/nominal#1265) still leads with "Create read and write token" and says Polylane "requests operational (edit) access", while the CLI now mints observers only — the PR body's callout is the honest way to hand that to a human. Worth settling soon, since a docs-page reader and a CLI user currently get opposite impressions of what Polylane can do to a Cloudflare account.
Nits (non-blocking):
- The PR body's "After:" paragraph is stale on two points: it still describes the superseded copy ("the link opens Cloudflare's account API token screen with a pre-filled, read-only token, create it as-is") and still says the
--read-onlyflag is "gone", contradicting the body's later no-op section. - The dropped "starts with cfat_ / shown only once" facts remain absent from the terminal copy — fine if intentional; the docs page still carries them.
- connect.ts:483-486 says "accepted and ignored for one release" — worth a tracking note so the removal actually happens next release rather than fossilizing.
Generated by Claude Code
| // Retired: Cloudflare now always connects read-only, which is what anyone | ||
| // passing this flag was asking for. Accepted and ignored for one release so | ||
| // existing scripts do not start exiting 2 on an unknown flag. | ||
| { flag: '--read-only', description: 'Deprecated: Cloudflare connects read-only either way; accepted and ignored', type: 'boolean' }, |
There was a problem hiding this comment.
Nit: the deprecation window says "one release" — consider filing the follow-up (or a TODO with the target version) now so the removal doesn't fossilize.
| // Always read-only. The docs page offers two pre-filled tokens (read+write | ||
| // first, read-only second), so the copy has to name the read-only one by | ||
| // its button label: a token minted from the other link and pasted here | ||
| // would be stored under a read-only label it does not have, and every | ||
| // write for the account would then be refused with no way to re-enable it. | ||
| // Read-only is also what both console connect surfaces send. |
There was a problem hiding this comment.
No action needed — this comment block is exactly the context the next editor needs. Nice.
There was a problem hiding this comment.
Auto-approved: Claude reviewed this PR and posted an LGTM verdict (see its review). A repo admin enabled this via the auto-approve workflow.
Generated by Claude Code
Review nit: "accepted and ignored for one release" had no deadline. The comment now names the release that added the no-op (0.2.16) and the one that removes it (0.3.0). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Thanks — all three nits addressed in 98bd126 and a PR-body edit:
On the flake: appreciate you chasing the 267-vs-272 gap to And agreed on the docs divergence being the thing to settle soon — a docs reader and a CLI user currently come away with opposite impressions of what Polylane can do to a Cloudflare account. It is flagged in the PR body and in #pr-reviews; not something I should decide by conflict resolution. |
There was a problem hiding this comment.
LGTM: the delta since the round-3 approval at bdc4c9b is exactly the two accepted nits — the --read-only no-op comment now carries both ends of its lifetime ("Retired in 0.2.16 … delete this entry in 0.3.0", connect.ts:483) and the PR body's After paragraph now matches the shipped copy and the no-op — a comment-only hunk with no behavior change.
Verified at 98bd126: git diff bdc4c9b..98bd126 is one comment hunk; CI 3/3 green (Node 20/22/24); locally typecheck, lint, and build clean. The suite reported 267/267 (0 failures) here — chased down rather than assumed: the round-3-approved bdc4c9b reports the same 267 in this environment, and the diff can't touch tests, so it's the known output.test.ts capture flake, not this push. Declining the cfat_ nit for verbosity reasons is fair — Cloudflare's UI states both facts at copy time.
Docs-page divergence remains the tracked open question (PR body + #pr-reviews), unchanged by this push.
Generated by Claude Code
There was a problem hiding this comment.
Auto-approved: Claude reviewed this PR and posted an LGTM verdict (see its review). A repo admin enabled this via the auto-approve workflow.
Generated by Claude Code
From the 2026-08-19 demo feedback on onboarding. Three things in
polylane cloud connectread as friction in the walkthrough: the provider multi-select made the enter key look dead, the Cloudflare step spent four sentences on permissions the user is told not to touch, and it then asked a write-permissions question that no longer has an answer.Before:
--multi(shipped yesterday in #43) opened a checkbox list of all nine providers pre-checked from repo markers, where space toggles and enter confirms — a user who pressed enter on the highlighted row got nothing selected. The Cloudflare token step explained account-owned tokens, Super Administrator, the pre-filled permission links, the read-only variant, and thecfat_prefix in one paragraph, then asked "Does the token have write permissions?" and sentreadOnlyaccordingly.After: the picker is a plain single select again — arrow keys and enter, one provider per run — and the installer's "Connect another cloud account?" loop is what covers a multi-cloud stack. The Cloudflare step says the one thing the user has to do: on the docs page, use the "Create read-only token" link, which opens Cloudflare's account API token screen with a pre-filled, read-only token; create it as-is. The write-permissions question is gone and the CLI always connects Cloudflare read-only, which is what both console connect surfaces already send (
CloudflareConnectSlideover.vueandCloudflareForm.vuehardcodereadOnly: true). The--read-onlyflag stays as an accepted no-op for one release rather than being removed outright — see below.Cloudflare is read-only from the CLI now — the docs page still leads with write
The docs page this step links to renders two pre-filled token links, and #1265 (merged this afternoon) put "Create read and write token" first. This PR's copy names the "Create read-only token" link by its button label so following the instruction literally produces the token the command stores, but the two sides now state different postures: the docs page says Polylane "requests operational (edit) access so agents can act on your infrastructure", and the CLI mints observers only. Someone should settle which is right — either the docs page follows the CLI to read-only-first, or the CLI grows a way back to a write token. Not resolving it here, because it reverses a decision made on purpose today.
Two behavior changes worth a release note: a scripted
--provider cloudflare --token Xconnect flips from the server default (readOnlyfalse) to read-only, and--read-onlyis kept as an accepted no-op for one release rather than being removed outright, so existing scripts do not start exiting 2 on an unknown flag. The no-op is dated in the code — added in 0.2.16, deleted in 0.3.0 — so it does not fossilize.Verified:
tsc --noEmitclean, eslint clean on both touched files,npm test272/272 pass,npm run buildemits.Tracking: coreplanelabs/nominal#1165 (unified onboarding). Pairs with coreplanelabs/polylanedotcom#128 and coreplanelabs/nominal#1282 from the same feedback round; off
main, no stack position.