Repository navigation
docs(minipay): say there is no sign-in step - #2357
Conversation
GigaHierz
left a comment
There was a problem hiding this comment.
head 1fb64c7 · API headRefOid == local refs/remotes/pr/2357 (no STALE); detached checkout; 7 commits behind main (92b8cc6), 1 ahead; main did not touch the overview page, so no conflict; mint broken-links --check-redirects: success; check-orphans: none; CI checks: all pass (Mintlify deploy skipped); primary sources fetched from docs.minipay.xyz
Verdict: REQUEST-CHANGES
Findings (severity-ordered)
-
MEDIUM · build-on-celo/build-on-minipay/overview.mdx:98 · The no-sign-in claim sits in the middle of the page, under a heading that is about the connect button, not in the first paragraph (line 7/9) or the
description(line 4). Failure scenario: an agent or search result chunks on the description or intro, or retrieves the "Stablecoins are the only assets" section for a question about authenticating users, and never sees the claim; the partner question that motivated the PR is still not answered where it is asked. AGENTS.md section 8 rules 1 and 3 say the Celo/MiniPay primitive belongs in the first paragraph. The new paragraph also splits the code block from the "Check forwindow.ethereum..." paragraph that belongs with it. Fix: add one clause to the line 9 paragraph (for example "Users are connected automatically; there is no connect button and no sign-in step"), and either rename the heading at line 74 ("Detect MiniPay, skip the connect button and sign-in") or move the sentence next to the "Hide your connect-wallet UI" sentence at line 76. -
MEDIUM · PR body,
Closes #1188· The issue asks for step-by-step MiniPay authentication guidance (flows, examples, diagrams). The PR adds one negative sentence. It does not say what an app does instead (read the connected address from the injected provider, and what that does and does not prove to a backend), and the MiniPay docs do not say either, so it should not be invented. Failure scenario: merge auto-closes #1188 and the guidance request disappears while a partner asking "how do I authenticate users?" still gets only "don't". AGENTS.md section 9:Closesonly when every acceptance box is met. Fix: change toRefs #1188, and state in the body that the remaining gap is the positive guidance (what identifies a user, backend-verification story), ideally with a filed follow-up or a question to the MiniPay team. -
LOW · overview.mdx:98 · Wording is imprecise. "either" has no clear antecedent (a code block sits between it and the connect-button sentence), "its wallet-connection guide rules out sign-in prompts" is a vague attribution (the upstream sentence is specific: "Do not prompt users to sign a message to access your site or authenticate; connection is automatic"), and the page never says that transaction signing is unaffected. Failure scenario: a reader concludes MiniPay never asks for any signature and removes legitimate transaction or typed-data signing flows, or does not know what "sign-in step" refers to. Fix, suggested text: "MiniPay connects the wallet when the page loads, so there is no sign-in step. Do not ask users to sign a message to access your app or to authenticate. Transactions are still signed in the wallet as usual." Keep the existing link to the Wallet connection guide.
Verified good
- Claim matches primary sources. docs.minipay.xyz wallet-connection page: "Do not prompt users to sign a message to access your site or authenticate; connection is automatic." The best-practices page has a "No message signing for access" section saying the same; the PR body quote is verbatim. The FAQ and Mini App overview also say connection is automatic with no Connect button.
- Nothing in the repo contradicts it. Grep for sign in / login / authenticate / connect in build-on-minipay, home/wallets.mdx and tooling/wallets: the only hits are the ODIS "Authenticate" comment and
authenticationMethodin the SocialConnect snippet (issuer auth to ODIS, unrelated to end-user sign-in), and a generic social-login remark on tooling/wallets, which is not MiniPay-specific. - The PR adds no new link or anchor. The existing Wallet connection link
https://docs.minipay.xyz/getting-started/wallet-connection.htmlreturns 200 (the extensionless form returns 404, so.htmlis correct and consistent with the page's other links). - Frontmatter, section order (Prerequisites ... Related), sentence-case headings, "Mini App" casing, no emoji, no promotional wording: unchanged and compliant. The edit adds no heading.
- The assistant issues #2314, #2320, #2324 and #2343 are open weekly digests; none contains a MiniPay sign-in or authentication question, so they neither support nor contradict the change. #1188 is open and unanswered.
- Rendering:
mint devserved the page (HTTP 200, title "Build for MiniPay - Celo Docs") and the console showed 0 errors at 1280x900. The 412x915 render was not verified.
What the PR got right
- Right fact, right source: it states the upstream rule, adds no invented API detail, no new links, no comparison, and stays within one concern in one file.
- Avoids restating a MiniPay fact on a second page beyond a one-line pointer to the canonical MiniPay guide.
- Follows the repo's own note that anchors/links must be hand-checked by adding none. The body includes the
mint broken-linksresult. - This is one round of targeted fixes (move/promote the sentence, tighten the wording,
Refsinstead ofCloses), not a rethink.
Merge-order hazards
- None blocking. Main moved 7 commits (including home/wallets.mdx, tooling/wallets/index.mdx and operator pages) but none edit build-on-celo/build-on-minipay/overview.mdx; the three-dot diff against main is only the PR's two added lines. Any other open PR editing the line 9 intro or the line 74-100 section of this page would conflict with fixes 1 and 3.
1fb64c7 to
62f4e75
Compare
|
All three are in 62f4e75, rebased onto current main.
|
GigaHierz
left a comment
There was a problem hiding this comment.
head 62f4e75 · fetch matched headRefOid (API == refs/remotes/pr/2357); branch 4 commits behind main (main already has the learn/ move, #2367; no build/ move yet), merge-tree clean; on a merge of head into main a12466b: mint broken-links --check-redirects pass, check-orphans.sh pass, CI green (broken-links, GitGuardian, StepSecurity); mint dev on :3413: page renders at 1280 and 412x915 with no horizontal scroll, 0 console errors at 412x915, and at 1280 after one transient cold-start 404 on the first load that did not reproduce in three further loads (no failing response seen)
Verdict: REQUEST-CHANGES
Scope: one file, build-on-celo/build-on-minipay/overview.mdx, +3/-1: a clause appended to the second intro paragraph (line 9) and a new paragraph at line 76 under "Detect MiniPay and skip the connect button".
Status of earlier findings
- No-sign-in claim buried mid-page and splitting the code block from its paragraph: FIXED. overview.mdx:9 now ends "Users are connected automatically; there is no connect button and no sign-in step." The new paragraph is at line 76, directly before the "Inside MiniPay the wallet is already connected..." paragraph, and the code block plus the "Check for
window.ethereum..." paragraph are adjacent again (lines 78-101). Heading kept; I accept that, since the intro now carries the claim. (The wording of the intro clause has a separate defect, new finding 1.) Closes #1188while only a negative sentence is added: FIXED. The body now saysRefs #1188and names the remaining gap (what identifies a MiniPay user to an app, how a backend verifies it without a sign-in signature).- Imprecise wording: FIXED as to the three points raised ("either" removed, attribution made specific, transaction signing mentioned). The wording I suggested then has two problems I did not check at the time; see new findings 1 and 2.
New findings
- MEDIUM · build-on-celo/build-on-minipay/overview.mdx:9 and :76 · Both additions say MiniPay does the connecting ("Users are connected automatically", "MiniPay connects the wallet when the page loads"). MiniPay's Wallet connection guide puts that duty on the app: "MiniPay requires that Mini Apps automatically connect to the wallet when the page loads", with a
useAutoConnect()hook that callsconnect({ connector: ... })on mount; MiniPay only injectswindow.ethereum. The page's own code block at lines 80-97 agrees with the guide, it callsconnect({ connector: injected(...) })in auseEffect, so the new prose contradicts the code directly beneath it. The PR body also says both additions "restate MiniPay's own Wallet connection guide"; only the "do not prompt users to sign a message" half does.- Scenario: a developer reads "MiniPay connects the wallet when the page loads", drops the
connect()call, anduseAccount()reports disconnected inside MiniPay, so the app shows an empty state or never sends transactions. - Fix shape: attribute the action to the app and keep the no-sign-in point. Line 76: "Your app connects automatically on page load, so there is no connect button and no sign-in step. Do not ask users to sign a message to access your app or to authenticate." Line 9: "Mini Apps connect to the wallet automatically on load; there is no connect button and no sign-in step."
- Scenario: a developer reads "MiniPay connects the wallet when the page loads", drops the
- LOW · build-on-celo/build-on-minipay/overview.mdx:76 · "Transactions are still signed in the wallet as usual" has no upstream source. The Wallet connection page says nothing about transaction signing, the Send a Transaction page (docs.minipay.xyz/technical-references/send-transaction) does not address user confirmation, and the Best Practices page only mentions "standard wallet prompts" in passing. The body presents this paragraph as a restatement of MiniPay's guide. This wording was my earlier suggestion and I had not verified it.
- Scenario: a reader treats an unsourced behaviour claim about wallet UX as MiniPay-confirmed and builds a flow around a confirmation prompt that MiniPay may present differently.
- Fix shape: drop the sentence, or keep it only with a link to a MiniPay page that states it. If dropped, the paragraph ends at "...or to authenticate", which is the sourced claim.
Verified good
- Upstream sentence is verbatim: docs.minipay.xyz Wallet connection, "Do not prompt users to sign a message to access your site or authenticate; connection is automatic." The Best Practices page repeats it ("...or to authenticate"). "Never show a 'Connect wallet' button" is also stated there, so "no connect button" is sourced.
- Wallet connection link at line 101 (
https://docs.minipay.xyz/getting-started/wallet-connection.html) is unchanged and returns content; no new links or anchors added; no heading changed, so no inbound anchor breaks. - Frontmatter, section order, sentence-case headings, "Mini App" casing unchanged; no emoji, no promotional wording, no roadmap language in the added text.
Refs #1188is the correct keyword; the body states what is not covered rather than inventing an answer.- Rendered output at 1280 and 412x915 shows both added sentences, h1 "Build for MiniPay", scrollWidth equals viewport width.
What the PR got right
- Right fact from the right source, with a verbatim upstream quote in the body, no invented API detail, one concern in one file.
- Responded to every earlier finding with a commit, kept the code block and its paragraph together, and narrowed the issue keyword honestly.
Remaining work is one wording correction (finding 1) and one sentence to source or drop (finding 2), not a rethink.
Merge-order hazards
62f4e75 to
ab70585
Compare
|
Both fixed in ab70585, rebased on main. The intro and the connect section now say the app connects on page load, which matches the guide and the |
GigaHierz
left a comment
There was a problem hiding this comment.
head ab70585 · API headRefOid == refs/remotes/pr/2357 after force re-fetch; 3 commits on a12466b, 1 file (+4/-2); git merge-tree --write-tree origin/main (7b8f97c, #2368) is clean, git follows the rename build-on-celo/build-on-minipay/overview.mdx -> build/mini-apps/overview.mdx; on a trial merge into current main: mint broken-links --check-redirects no broken links, scripts/check-orphans.sh no orphan pages; gh pr checks: broken-links, GitGuardian, StepSecurity pass, Mintlify Deployment skipping; MiniPay wallet-connection.md, best-practices.md and llms-full.txt fetched and compared
Verdict: APPROVE
Status of earlier findings
Earlier review was at 62f4e75 (newest of two).
- Intro and section wording said MiniPay does the connecting ("Users are connected automatically", "MiniPay connects the wallet when the page loads"), contradicting MiniPay's Wallet connection guide and the page's own
connect()code block: FIXED. The intro now reads "Mini Apps connect to the wallet automatically on page load; there is no connect button and no sign-in step." (line 9) and the section opens "Your app connects to the wallet automatically when the page loads, so there is no connect button and no sign-in step." (line 76 after the move). The app is the subject in both, as in the guide: "MiniPay requires that Mini Apps automatically connect to the wallet when the page loads." - Unsourced sentence "Transactions are still signed in the wallet as usual": FIXED. The sentence is gone from the diff. The paragraph now ends at "...or to authenticate", which is sourced.
- (Second half of finding 1) The paragraph above the code block said the wallet was "already connected": FIXED. It now reads "Inside MiniPay the host injects the provider as
window.ethereum, andwindow.ethereum.isMiniPayistrue. Connect the injected connector on load:". MiniPay's guide: "MiniPay injectswindow.ethereumwhen it loads your app" and "Checkwindow.ethereum?.isMiniPay === trueto detect MiniPay."
Findings
None.
Verified good
- Every changed statement against MiniPay's own docs (docs.minipay.xyz Wallet connection, Best Practices,
llms-full.txt):- "no connect button": "never show a 'Connect wallet' button" and "Never show a connect button" (Wallet connection; Best Practices "Always auto-connect").
- "connects ... automatically on page load": "the wallet connection must happen automatically on page load" and the
useAutoConnecthook that callsconnect()in a mount effect. - "Do not ask users to sign a message to access your app or to authenticate": "Do not prompt users to sign a message to access your site or authenticate; connection is automatic."
- "no sign-in step": no page in the MiniPay index (llms.txt) describes a sign-in or authentication step; the only authentication mention is the "No message signing for access" rule.
- "the host injects the provider as
window.ethereum" andisMiniPay: both stated on Wallet connection and "How Mini Apps run".
- No unsourced claim remains in the diff: the only added statements are the ones above. The PR body's two quotes are verbatim from the Wallet connection page (bold markup aside).
- The code block directly below the new paragraph (
connect({ connector: injected(...) })insideuseEffect) agrees with the prose; the intro and the section statement are consistent with each other. - Link to Wallet connection (
https://docs.minipay.xyz/getting-started/wallet-connection.html) returns 200; no heading changed, so no anchor breaks; no new links. Refs #1188is the right keyword: the issue is open and asks for authentication guidance; the body states the remaining gap (what identifies a MiniPay user to an app, how a backend verifies it without a sign-in signature), and MiniPay's docs cover neither.- AGENTS.md rules: frontmatter and section order unchanged, "Mini App" casing, "app" not "dApp", no emoji, no promotional or roadmap wording, no blockquote callouts.
- Smoke render of the trial merge at
/build/mini-apps/overview: 200, old path/build-on-celo/build-on-minipay/overview307 to it, both added sentences present at 1280 and 412 wide, no horizontal scroll, no console errors. Smoke test only.
What the PR got right
Right fact from the right source with the upstream sentences quoted in the body, the action attributed to the app instead of to MiniPay, an unsourced sentence dropped instead of defended, a narrowed Refs instead of Closes, and one concern in one file.
Merge-order hazards
- #2368 is on main; the merge is clean through the rename, so no rebase work is required (a rebase is optional, only to show the new path).
- Pairwise
git merge-treewith #2346 and #2355: clean. No other open PR in the current list was found to touch the MiniPay overview page by title; not checked file by file. - Observations, not findings against this PR (both predate it and sit outside the diff): (a) MiniPay's Best Practices page says "MiniPay connects automatically" in one sentence while its Wallet connection page says the app must connect; this PR follows Wallet connection and the code. (b) The code block's trailing comment
// render <ConnectButton /> only when falsedescribes an app that also runs outside MiniPay, while MiniPay's guide says to show a "run inside MiniPay" message there instead of a connect button. Worth a follow-up issue if the owners want the page fully aligned.
…laim MiniPay's Wallet connection guide puts auto-connect on the Mini App, and the code block below already calls connect(). No MiniPay page states how transaction signing works, so that sentence goes.
ab70585 to
f22bfbd
Compare
GigaHierz
left a comment
There was a problem hiding this comment.
Re-approving after a rebase onto the new build/ layout (f22bfbd). The content is the head I approved (ab70585): the same file changes, and the only difference is the page path (build-on-celo/build-on-minipay/overview.mdx is now build/mini-apps/overview.mdx). mint broken-links --check-redirects and the orphan check pass on the rebased branch.
GigaHierz
left a comment
There was a problem hiding this comment.
Re-approving after a rebase onto the new build/ layout (f22bfbd). The content is the head I approved (ab70585): the same file changes, and the only difference is the page path (build-on-celo/build-on-minipay/overview.mdx is now build/mini-apps/overview.mdx). mint broken-links --check-redirects and the orphan check pass on the rebased branch.
MiniPay's overview page explained auto-connect but never said there is no sign-in step, so partners kept asking how to authenticate MiniPay users (#1188). This adds a clause to the intro saying Mini Apps connect to the wallet automatically on page load, with no connect button and no sign-in step, and a short paragraph under "Detect MiniPay and skip the connect button" telling apps not to ask users to sign a message to access the app or to authenticate. Both follow MiniPay's Wallet connection guide, which the page already links. That guide makes auto-connect the app's job, and the code block in the same section already calls
connect()on load. The paragraph above that code block said the wallet was "already connected"; it now says MiniPay injects the provider, so it no longer contradicts the code.From Wallet connection: "MiniPay requires that Mini Apps automatically connect to the wallet when the page loads. Never show a connect button." and "Do not prompt users to sign a message to access your site or authenticate; connection is automatic."
Refs #1188. The issue stays open for the positive guidance: what identifies a MiniPay user to an app, and how a backend verifies that user without a sign-in signature. MiniPay's docs cover neither, and I didn't want to invent an answer here.
mint broken-links --check-redirects: success no broken links found.bash scripts/check-orphans.sh: No orphan pages found.