feat(config): hints — a first-class switch for next-step guidance - #58
Conversation
hints is git's advice.* category for this CLI: human guidance about what to do next, distinct from data (stdout contract), status lines, errors, prompts, and consent notices, which it never gates. Resolved env (POLYLANE_HINTS) → config file → default on, same boolean model as telemetry. First consumers: the post-sign-in next-steps boxes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
0c640d6 to
6383cd9
Compare
There was a problem hiding this comment.
LGTM: Correct implementation of the hints capability — env/config precedence matches telemetry, underInstaller()/POLYLANE_ONBOARDING_RUN is cleanly split back to attribution-only with no orphaned references, and the two nextSteps() gates are the only call sites touched, consistent with the PR's stated scope.
Verified (procedure per REVIEW.md):
npm ci && npm run codegen && npm run typecheck && npm run lint && npm testall clean locally; 306/306 tests pass (matches the PR's claim). CI is green on Node 20.x/22.x/24.x (checks / Test & typecheck).POLYLANE_HINTSenv →hintskey in~/.polylane/config.json→ defaulttrue, same precedence shape astelemetry(src/config/loader.ts:104-112).underInstaller()is fully deleted. The only remaining uses ofPOLYLANE_ONBOARDING_RUN(src/auth/onboarding-run.ts,src/commands/auth/signup.ts:267,283) are the one-shot attribution ride-along on the signup POST body — never read as a boolean UX gate anywhere in the tree. Matches the PR's stated intent exactly.- Both new
config.hintsgates (src/commands/auth/signup.ts:217,303) wrap only the "Next steps" guidance box. The JSON data path (emitResult(...)) above and theoutro(...)status line below each stay unconditional — no data/status/error/prompt output got swept in.
Nits (non-blocking):
nextSteps()'s first line ("Signed in. Session valid until<expiresAt>") is status-shaped content living inside the now-suppressible box — a small drift from the PR's own "never gates status" framing. Not an actual data-loss regression:polylane auth status(src/commands/auth/status.ts:64-72) exposes the same expiry unconditionally regardless ofhints. Still, worth hoisting that line above theif (config.hints)check (or dropping it, sinceauth statusalready covers it) in a follow-up.- The new README line documents
0/false/offas disabling values forPOLYLANE_HINTS, butparseEnvBoolean(src/config/loader.ts:40-43) only treats the exact lowercase strings'','0','false','no'as false —off(and any uppercase variant) actually leaves hints enabled. This is copy-pasted from the pre-existingPOLYLANE_TELEMETRYREADME line, which has the identical mismatch, so it's a consistent (if incorrect) existing pattern rather than a new bug — but worth fixing both together. skill/SKILL.md's "Configuration precedence" table (baked intosrc/generated/skill.tsat build time) still doesn't listPOLYLANE_HINTS, unlikeREADME.mdwhich this PR updates. Minor doc-completeness gap for agents relying on the skill guide; doesn't affect runtime behavior and this diff doesn't touch build-related files so no build/smoke leg was required.
None of the above bear on the correctness of the core gating logic or the underInstaller() removal.
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
…eview parseEnvBoolean accepts 'off' and any casing, matching what the README documents for POLYLANE_TELEMETRY and POLYLANE_HINTS. nextSteps drops its status-shaped expiry line (auth status owns that). SKILL.md gains the POLYLANE_HINTS row. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
All three nits taken:
306/306 tests, typecheck + lint clean. |
The feature
hintsis a new boolean config capability naming a category of output this CLI already produces but had no switch for: next-step guidance for humans — the post-sign-in "Next steps" box, and future "connect any time later"-style nudges. It is defined by what it never touches: data (the stdout JSON contract), status lines ("✓ connected"), errors, prompts, and consent notices (the terms notice is legal text, not a hint).How it works
Same model as
telemetry:POLYLANE_HINTSenv →hintsin~/.polylane/config.json→ default on. Managed viapolylane config set --key hints --value offand visible inconfig show. No CLI flag yet — a per-invocation need hasn't shown up, and env covers orchestrators. First consumers: the twonextStepsboxes inauth signup, whose suppression switches from thePOLYLANE_ONBOARDING_RUNcheck (#57) to this key — an attribution channel shouldn't double as a UX switch, so the run id goes back to meaning attribution only.The research
Best-in-class CLIs model this as a capability toggle, never caller identity:
advice.*(~40 keys suppressing exactly this class of hint, data/status untouched) — the naming precedenthintsfollowsHOMEBREW_NO_ENV_HINTS; npm per-nag toggles (fund,update-notifier,audit); gcloud splits prompts from output (--quiet= never prompt, not less output); AWS CLI pager/auto-prompt as separate switchesCI=trueis the ecosystem's "an orchestrator is driving" preset that flips several such toggles — presets compose out of capabilities, not the other way roundAn orchestrator shouldn't declare who it is; it should declare what it wants: "no hints, keep status, keep prompts." Toggles with narrow semantics evolve independently; a documented "mode" freezes its bundle of behaviors forever.
How it will be used first
The install script owns the onboarding journey (connects, mapping, topology link), so it exports
POLYLANE_HINTS=0for its CLI invocations (polylanedotcom companion PR).306/306 tests, typecheck + lint clean.
🤖 Generated with Claude Code