Repository navigation
docs: fail on any relative docs path in a doc comment and complete the retry guidance - #8
Conversation
…e retry guidance - A stateful signer that cannot sign the same digest twice has its entry dropped from the retry's keys, or sign returns undefined for it, since a throw would stop the retry at the same plan again. The retry also drops every entry that can no longer help finalize the transaction: the other entries for a leaf that cannot complete without that key, and every entry when an input is left with no leaf that can complete. The README, the signing page and the P2mrPsbtSigningError TSDoc say so. - check-doc-links.mjs fails on a relative path into a docs directory anywhere in a doc comment outside code, in a link or in plain text, instead of trying to recognize each link form. A token is absolute only with // after an optional scheme, in any case. A self-test pins the rule. - The doc comments that still named a topic page by a relative path (p2mrSighash, three in the api client) link it by URL, and CONTRIBUTING describes the check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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. |
![qbit-reviewer[bot]](https://avatars.githubusercontent.com/in/5446813?s=60&v=4){kind=small}
| // repository, as a link target or as plain text, so a doc comment links a | ||
| // topic page by its URL instead. A bare `docs` or `docs#x` is a word or a | ||
| // declaration reference, not a path. | ||
| const RELATIVE_DOCS_PATH = /(?:^|[/=])docs\/|\/docs(?:[#?]|$)/i |
There was a problem hiding this comment.
1. [low · nit] RELATIVE_DOCS_PATH misses docs/ glued to ':' (e.g. [x]:docs/x.md) and a bare ../docs right before */
scripts/check-doc-links.mjs:34
Case 1: a doc comment line such as * [guide]:docs/x.md is a valid CommonMark link reference definition. So is Note:docs/x.md. URL_TOKEN (line 23) splits the first at ], giving the token :docs/x.md. TOKEN_TRIM (line 36) strips only *_~ from the front, so the : stays. RELATIVE_DOCS_PATH then needs start, / or = before docs/, so nothing is reported. Case 2: in /** pages in ../docs*/ the token is ../docs*/. Nothing gets trimmed and neither branch matches. The self-test only covers : ../docs and :../docs, where a / comes before docs. Either way, the check passes a relative docs path, which contradicts CONTRIBUTING's 'anywhere in a doc comment outside code'.
| const RELATIVE_DOCS_PATH = /(?:^|[/=])docs\/|\/docs(?:[#?]|$)/i | |
| const RELATIVE_DOCS_PATH = /(?:^|[^\w.-])docs\/|\/docs(?:[#?]|\*\/|$)/i |
| const ABSOLUTE = /^(?:[a-z][a-z0-9+.-]*:)?\/\//i | ||
| // A doc comment: a `/**` at the start of a line or after whitespace. One glued | ||
| // to other text, as in a glob string, is not. | ||
| const DOC_COMMENT = /(?:^|(?<=[ \t]))\/\*\*(?!\/)[\s\S]*?\*\//gm |
There was a problem hiding this comment.
2. [low · nit] DOC_COMMENT ignores doc comments glued to '(' or '{', a regression from the old every-line check
scripts/check-doc-links.mjs:42
DOC_COMMENT only accepts /** at the start of a line or after a space or tab. Take function f(/** see [x](../docs/x.md) */ a: number) or {/** ../docs/x.md */ a: 1}. TypeScript and TypeDoc attach these comments to the parameter or member, but the comment is never matched, so relativeDocsPaths never sees it (lines 184-185). The old RELATIVE_DOC_LINK ran on every line and flagged ](../docs/x.md there. The same boundary also takes a /** after whitespace inside a string or a // comment as a doc comment that runs to the next */, which could flag code as prose. Neither form occurs in packages/*/src today.
| .map(line => { | ||
| const prefix = /^(?:[ \t]*\/\*\*|[ \t]*\*(?!\/))?[ \t]?/.exec(line)[0] | ||
| const content = line.slice(prefix.length) | ||
| const marker = /^\s{0,3}(`{3,}|~{3,})(.*)$/.exec(content) |
There was a problem hiding this comment.
3. [low · nit] A line starting with a code span is taken as a fence opener, which blanks the rest of the comment
scripts/check-doc-links.mjs:56
Take the doc comment line * ```foo``` see ../docs/x.md. CommonMark does not let a backtick fence's info string contain backticks, so this line is an inline code span followed by prose. The marker regex at line 56 matches it anyway. Lines 58-60 then set fence, and since no bare closing run follows, that line and every later line of the comment are blanked as code. A relative docs path there is never reported.
| const marker = /^\s{0,3}(`{3,}|~{3,})(.*)$/.exec(content) | |
| const marker = /^\s{0,3}(`{3,}(?=[^`]*$)|~{3,})(.*)$/.exec(content) |
Summary
Fixes for the three nits qbit-reviewer posted on #7 after it merged, plus what local reviews of those fixes found:
keysand the other plans are then signed without it. That isn't safe when another entry depends on the dropped key. The README, the signing page and theP2mrPsbtSigningErrorTSDoc now give the full rule:Drop the signer's entry, or have
signreturnundefinedfor it. Throwing stops the retry at the same plan again.Also drop every entry that can no longer help finalize the transaction:
multi_aleaf left short of its threshold;Signing those spends key uses for nothing.
check-doc-links.mjsno longer tries to recognize each link form. It enforces one rule: in a doc comment, outside code, a relative path into adocsdirectory fails, as a link target or as plain text. Examples aredocs/x.md,../docs/x.md#a,./docs/and../docs, in any case, including inside emphasis or before sentence punctuation. Neither TypeDoc nor an IDE hover resolves such a path against the repository, so a doc comment links a topic page by its URL.docsordocs#xis a word or a declaration reference, so it passes./**at the start of a line or after whitespace, so a trailing/** … */after code is checked and a glob string such as'src/**'isn't.p2mrSighashand three in code spans in the api client. CONTRIBUTING describes the check.//after an optional scheme (HTTPS:,ftp:, any case), soNote:../docs/x.mdis still caught. Before, only lower-casehttp:andhttps:targets counted.Testing
packages/api/src/index.ts, wrapped across two comment lines, is reported at its file and line. A path in a code span next to it is not.pnpm format:check,pnpm docs:api:check(111 links) andpnpm typecheckpass.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.