Repository navigation
docs: Answer first-time questions in the user guide - #97
BenWestgate wants to merge 1 commit into
Conversation
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
AI-generated release-gate review, posted at the maintainer's request.
Concept ACK, but I would keep this draft for two documentation fixes before human review:
- “
ms32 create 2and the other thresholds write shares at random indices” is only true when indices are not explicitly supplied. The guide already documents--indices, so this answer should say the presets/default share-count path chooses random ordinary indices while--indicesuses the operator's explicit indices. - #90 explicitly says the length-specific cards from #88 show where the string ends. Once #88/#96 is settled, this answer should link the 48- and 74-character card templates (or state that the rarer lengths have no dedicated template yet) rather than leaving the length answer disconnected from that requested guidance.
The fingerprint/no-record and case answers match #57's reviewed contract. No security blocker beyond preserving that wording. The commit is Claude-authored and still requires responsible-human rewrite/squash before integration.
|
Reviewed the FAQ against the CLI. Follow-up 7fa8137 qualifies the two defaults: operators can choose share indices with --indices and specify an identifier. The original answers about the wallet-record fingerprint, length, and case remain unchanged. The updated documentation diff is ready for human review; the follow-up is agent-authored and should be handled under the repository authorship policy. |
BenWestgate
added
the
gate: adversarial review
ChatGPT Codex Connector
054e8d9 to
115f2c2
Compare
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Codex current-head follow-up review of 7fa8137: the first review finding is fixed. The share-index answer now correctly says the default ms32 create 2 path chooses random indices and points operators to --indices for explicit choices; the identifier wording likewise distinguishes the random default from an explicitly supplied identifier. Exact-head Python-package run 552 succeeded and there are no inline review threads.
One documentation blocker remains before ACK/human handoff: the length answer must link the 48-character standard recovery card and the 74-character/256-bit recovery card from #96, and explicitly say that 54/61/67/127-character backups do not yet have dedicated printable templates. Keep this draft until that follow-up is applied on the post-#96 base, then rerun the review. Claude/agent-authored commits still require responsible-human rewrite/squash before integration.
7fa8137 to
aafb716
Compare
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
AI-generated review (Codex), posted at the maintainer's request.
ACK aafb716. The added first-use guidance matches the implemented recovery model.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aafb7167db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Distinguish random shared-backup identifiers from the fingerprint-derived unshared default. Include the exact reviewed 256-bit recovery card from PR #96 so the new FAQ link resolves on this branch as well as the final integration. Address the two current PR #97 review findings. The card blob is identical to the independently print-previewed PR #96 artifact, and git diff --check passes. AI-assisted follow-up requested by the maintainer; refs #90.
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Codex follow-up review, generated by the agent at the maintainer's request; not a human approval or an independent review of my own edits.
ACK 1cf1a17793652ced53a7e5416ccb29d506782d60 for the two documentation follow-ups. The identifier explanation now distinguishes random shared identifiers from the unshared BIP32-fingerprint-derived default. The 256-bit recovery-card target exists and its blob is identical to #96's previously print-previewed artifact; the standard-card link also resolves. Explicit-index behavior, rarer lengths, typed-record/no-record restoration, and fresh creation remain qualified. Current-head GitHub Python-package checks have succeeded; both inline findings are answered and resolved. Integrate #96's standard-card change before this FAQ. Human review/authorship/signatures remain.
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Codex current-head review at 1cf1a17: no findings. The new FAQ accurately separates the wallet record/fingerprint from threshold cards and documents current card lengths/indices without changing recovery behavior.
A tester asked why the fingerprint stays off the cards, why restore asks for it, how long a string is, which share indices create uses, and whether letter case matters. Answer each in a short section. Shared sets get random identifiers; an unshared secret defaults to an identifier derived from its fingerprint. The length answer links the standard and 256-bit recovery cards. The 256-bit card is the same file as in #96, so the link resolves whichever lands first. Closes #90 Claude-Session: https://claude.ai/code/session_01Q1TNm1YeYW8K9W2eBvxmCD
1cf1a17 to
788137b
Compare
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
AI-assisted current-head documentation review: ACK 788137b. The FAQ matches the CLI's fingerprint, string-length, share-index, and casing behavior. No findings.
This branch has not been deployed
1 participant
The first-use guide explains wallet records, typed fingerprints, supported backup lengths, share indices, S, and letter case. It distinguishes random shared identifiers from the fingerprint-derived unshared default and includes the reviewed 256-bit recovery card so the FAQ link resolves on this branch.
The card blob is identical to #96's print-previewed artifact. Integrate #96's standard-card update first, then this FAQ. The two review findings are fixed on current head
1cf1a17: the identifier wording is qualified correctly, and the 256-bit card target exists while rarer 54/61/67/127-character backups are explicitly described as lacking dedicated printable cards.Current-head Python-package CI is green and Codex has ACKed the follow-up. Closes #90. AI assistance was used for the maintainer-requested follow-ups; human review/authorship and signed integration remain.