Repository navigation
README v2 and AGENTS.md: quickstart first, docs links, agent guidance - #338
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
SpertsyanKM
left a comment
There was a problem hiding this comment.
Thanks, Vlada — snippets, minimum versions, pins, links and the MCP destructive-tool flow check out against main. Changes needed before merge:
This repo
- AGENTS.md:59 (minor) —
Debug.Logneedsusing UnityEngine;(onlyusing QonversionUnity;is shown at l.40). - README.md:135 / AGENTS.md "Sample" (minor) — "ships no sample scene" + Flutter link: there is a public qonversion/unity-sdk-sample (Assets/Scripts/SampleApp.cs). Fine to link it.
- AGENTS.md:95 / README.md:105 — see the Apple Ads note above: write failures can also be the server-side switch or a missing Apple org role, not "beta not on".
- Requirements (Unity 2021.3, EDM4U 1.2.189, iOS 13.0, minSdk 23): OK. We'll bump
"unity"in package.json separately. - Repo description: OK.
Shared with the other README v2 / AGENTS.md PRs
- Apple Ads availability — my earlier guidance is outdated, sorry. After 2026-10-09, mcp-server #126 (DEV-1480) removed the separate write beta: there is no
apple_ads_writeaccount gate any more (mcp-server/internal/tools/apple_ads_write.go:134). Writes need theapple_adsaccount feature, theapple_ads:writescope, a dashboard member with campaign-management permission and Apple org write access, and can be switched off server-side. "Closed beta — ask support to switch it on" sends agents the wrong way. Until Sam finalises the Apple Ads docs, please use neutral wording, e.g.: "Apple Ads tools need the feature on your account (404 otherwise); campaign changes additionally need theapple_ads:writescope and a team member with campaign-management permission — checkget_apple_ads_connection_status." - AGENTS.md "If the three disagree, the docs page wins". For the version the docs page lags (iOS page says 6.17.0, Flutter 11.11.0, Unity 9.10.0 and "2018.3", Android
9.+, Cordova still lists cordova-ios 6.x; Web/RN pages name no version). Make the release tag / package registry authoritative for the version, docs for API usage, and tell the agent to report a mismatch. Same for the integration prompt line "use the SDK version the docs name".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@SpertsyanKM thanks for the review. All points are addressed in bae886e — could you take another look? This repo
Shared with the other README v2 / AGENTS.md PRs
Repo description is left to you, as agreed. Not merging from our side. |
SpertsyanKM
left a comment
There was a problem hiding this comment.
Re-reviewed bae886e — the round-1 points are fixed (using UnityEngine;, unity-sdk-sample link, Apple Ads, package.json 2018.3 lag note, version source via OpenUPM). Verified: tag = OpenUPM latest = 9.11.0; unity-sdk.git#9.11.0 exists; the sample features match SampleApp.cs.
Small things, all quick:
- README.md:6 — "lined up with what App Store Connect reports" is the iOS copy-paste leftover (fixed in Flutter and RN, I missed it here in round 1). Unity is iOS + Android. Suggested: "…lined up with what the App Store and Google Play report, …"
- AGENTS.md:110 (nit) — "
Environmentin the wrappers" isn't true for all wrappers (Flutter usesQEnvironment). Suggest: "In this SDK the enum isEnvironment(Runtime/Scripts/Dto/Environment.cs); read it from the SDK, not from memory." - README.md:136-137 / AGENTS.md:114 (optional) — the sample's
Packages/manifest.jsonloads the SDK fromfile:../../unity-sdk, so it only opens with this repo cloned next to it. Suggested addition: "It loads the SDK from a sibling checkout (file:../../unity-sdk), so clone this repository next to it."
Once these are in, it's good to merge from your side — no need for another round from me unless something else changes.
Shared across all nine README v2 / AGENTS.md PRs
- PR body before squash-merge. The body still says reading Apple Ads "is on for all accounts" and changing campaigns "is a closed beta per account". Both are outdated (mcp-server#126). The body becomes the squash commit message, so please update that sentence, or replace the commit body when merging, e.g.: "Apple Ads in MCP: tools need the
apple_adsaccount feature; campaign changes additionally need theapple_ads:writescope and campaign-management permission (mcp-server#126)." - "A refused Apple Ads call means missing access … do not retry" (AGENTS.md). Slightly too absolute:
get_apple_ads_connection_statuscan reportcampaign_management.state: unknown(permission snapshot not landed yet — try later), and an Apple authorization can need reauthorization. Suggested: "A refused Apple Ads call usually means missing access or an Apple authorization that needs attention, not a broken server: callget_apple_ads_connection_status, do not work around it, and tell the human what it reports." Non-blocking; can go together with Sam's final Apple Ads wording.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@SpertsyanKM round 2 is done in bca5358:
Ready to merge. We only have read access here, could you merge it? |
SpertsyanKM
left a comment
There was a problem hiding this comment.
Re-reviewed bca5358: all round-2 points are fixed and verified against source. Thanks! Merging.
Summary
Files:
README.md,AGENTS.md.checkEntitlements/entitlements). The mode is named out loud, with a link to Analytics Mode.llms.txt, the Qonversion MCP server with the Apple Ads access conditions, the two-step confirmation for destructive tools, and the integration prompt (the docs text, with step 3 taking the SDK version from the release tag or the package registry).AGENTS.mdfor coding agents: how to get the current version instead of one from memory, install, init, first call, docs as markdown, MCP, and what an agent must not do without the human saying yes (keys, mode, live project configuration, destructive MCP tools, environment).Removed on purpose: the Analytics / A/B Experiments / Integrations / Why Qonversion? sections (they duplicate the website and push the install below the fold); long platform guides → a link into the docs; all images except the header.
The OpenUPM badge is added.
Facts checked
Minimum versions, from this repository's manifests:
unityfield still says 2018.3, which is not supported or tested, and the README says so in one line —package.jsonminSdkVersion23 (inherited from the native SDKs)Init and first-call signatures:
Runtime/Scripts/QonversionConfigBuilder.cs,Runtime/Scripts/Qonversion.cs,Runtime/Scripts/IQonversion.cs,Runtime/Scripts/Dto/Entitlement.cs,Runtime/Scripts/Dto/LaunchMode.cs.Versions:
openupm addcarries no version. The Git URL example uses tag 9.11.0 (latest release on 2026-10-12) with "current at the time of writing; take the latest tag from Releases".Header image
The approved header (logo and platform list, no positioning text) ships in this PR as
.github/assets/sdk-header.png, referenced by a one-line<img>tag on line 1 with a relative path, which GitHub, npm and pub.dev all resolve. The quickstart line count does not move.Links
All 32 URLs in these files checked with
curl -sI -Lon 2026-10-12: 30 return 200, the rest are expected:401https://mcp.qonversion.io/mcp — OAuth endpoint of the MCP server, expected without a token405https://documentation.qonversion.io/mcp — the docs MCP endpoint does not answer GET, it speaks MCPLICENSEexists in this repository (hygiene PRs merged 2026-10-09/10).docs/for-agentsis live.Review
Four independent reviews in the marketing hub (product, marketing, QA, brand), two iterations. Maintainer answers of 2026-10-09 applied here: Apple Ads in MCP: tools need the
apple_adsaccount feature; campaign changes additionally need theapple_ads:writescope and campaign-management permission (mcp-server#126); Unity 2021.3 as the floor (2018.3 dropped); Git URL tag 9.10.1 → 9.11.0; thediscard_*docs-gap note removed fromAGENTS.md.Reviewer: @SpertsyanKM — the review request cannot be set from a fork, hence the mention.
Repository description (settings, not a file)
The repository description shown under the name on GitHub is a setting, so it cannot be part of this PR. Proposed text to set when merging, so the description, the README header and the docs say the same thing:
Current: Unity SDK for cross-platform in-app purchase and subscription infrastructure, revenue analytics, engagement automation, and integrations
One command for a maintainer:
gh repo edit qonversion/unity-sdk --description "Qonversion Unity SDK: iOS and Android purchases confirmed with the stores, entitlements, and the revenue data behind Apple Ads, A/B tests and analytics.".Maintainer review rounds of 2026-10-13 and 2026-10-14 applied (see the PR comments).
🤖 Generated with Claude Code