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.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
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.