Repository navigation
fix: [SDK-5575] reword the checkpoint consent question in plain words - #78
Merged
Merged
Conversation
1 of 5 tasks
sherwinski
force-pushed
the
sherwin/sdk-5575
branch
2 times, most recently
from
October 19, 2026 23:46
934658c to
279a63f
Compare
sherwinski
added this pull request to stack #67
October 19, 2026 23:49
sherwinski
commented
Oct 20, 2026
abdulraqeeb33
approved these changes
Oct 22, 2026
abdulraqeeb33
left a comment
abdulraqeeb33
left a comment
Contributor
There was a problem hiding this comment.
Approving. The "never contains sensitive data" line is backed by the §16 filters in checkpoint.sh, so the shorter question doesn't overclaim. Two small notes inline.
| fold it into the network-access request. The question says what the checkpoints | ||
| are for, names the host (`api.onesignal.com`), and says what a checkpoint never | ||
| contains (source code, credentials, or other sensitive data). It does not list the | ||
| payload fields: a first-time user needs the purpose, not the schema. The field list |
Contributor
There was a problem hiding this comment.
Non-blocking, for the wording sign-off: dropping the field list also drops the one fact a user might weigh, that each report carries their App ID and so ties back to their OneSignal app. The old question said so. If the team wants consent to cover that, one clause would do it without bringing back the schema.
sherwinski
force-pushed
the
sherwin/sdk-5575
branch
from
October 22, 2026 20:42
0044200 to
3badd8b
Compare
Dogfooding testers did not know what a checkpoint is or why the data goes to `api.onesignal.com`: the question opened with the word and the field list. The question in the setup, credentials, and verify skills now opens with the purpose, defines a checkpoint, and then names the fields, the host, and what never leaves the machine (safety contract §15). The setup skill's version also names the host now; it did not before. The 3 skills share one text, and the choice labels do not change.
…EADME's status words - Safety contract §15 now says the consent question opens with what the data is for, before the field list. The 3 skill copies repeat the rule, but the contract is where a rule lives. - The question says "success or fail", the words the README uses for the same field, instead of "pass or fail".
…eld list The consent question now reads: "This plugin can send reports to OneSignal (via api.onesignal.com) to help our team understand where setup fails for developers and fix it. A report never contains source code, credentials, or other sensitive data. Share these reports with OneSignal?" The choices are "Share reports with OneSignal" and "Keep reports on this machine only". - Safety contract §15 no longer asks the question to list the payload fields; §16 and the README keep the list for the user who wants it. - §14 and the README quote the new choice labels, and the Codex plain-text example reads "Reply share or keep". - The 3 skills record `1` for "share" instead of "send".
…ts" is the user-facing word
The sentence named the answer "send" after the labels changed to "share". The setup copy already used "share" in both places.
sherwinski
force-pushed
the
sherwin/sdk-5575
branch
from
October 22, 2026 20:51
3badd8b to
c2fedc2
Compare
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.
One Line Summary
The checkpoint consent question in the
setup,credentials, andverifyskills now says what the reports are for, names the host, and says what a report never contains; it no longer lists the payload fields.Motivation
In the second dogfooding session, 2 testers (one in Claude Code, one in Codex) saw the consent question as the first thing the plugin asked and did not know what a checkpoint is or why the data goes to
api.onesignal.com. The old text opened with the word "checkpoints" and a field list. The question also differed between the skills: thesetupversion did not name the host, which safety contract §15 requires.Ticket: SDK-5575, sub-issue of SDK-5556. Stacked on #77 (SDK-5574), which sits on #66.
Scope
skills/setup/SKILL.md,skills/credentials/SKILL.md,skills/verify/SKILL.md: one shared question text: "This plugin can send reports to OneSignal (viaapi.onesignal.com) to help our team understand where setup fails for developers and fix it. A report never contains source code, credentials, or other sensitive data. Share these reports with OneSignal?" Choices: "Share reports with OneSignal" / "Keep reports on this machine only". The skills record1for "share".references/safety-contract.md: §15 no longer asks the question to list the payload fields (§16 keeps the list); §14 quotes the new first choice and the Codex plain-text example reads "Replyshareorkeep".README.md: quotes the new "Keep reports on this machine only" label.Not changed: the skip rules, the file write, the
.onesignal/telemetryvalues, andcheckpoint.sh. The final wording is pending team sign-off.Testing
python3 scripts/package_directory_bundle.py --check: pass.Affected code
skills/setup/SKILL.mdskills/credentials/SKILL.mdskills/verify/SKILL.mdscripts/references/Checklist