feat(check): add the check command - #119
Merged
Merged
Conversation
Extract's leniency for a "---" that never closes -- falling back to "no frontmatter, whole file is body" -- hid a common paste mistake completely, including from every command that calls Parse (create surfaced it as a confusing "missing title" error instead). Parse now reports ErrUnterminatedFrontmatter; linkindex.Build skips the offending file the same way it already skips an unreadable one, so one malformed sibling never blocks checking or converting an unrelated file.
Index.FileExists answers "does this file exist under root" without regard to whether it has a page_id yet -- reusing the anchors map, which Build already populates (even to an empty map) for every walked .md file, unlike pages, which only gets an entry once a file is published. This is what lets convert's upcoming link-severity split tell "missing entirely" apart from "exists but not published yet".
resolveDocKey now reports whether a link/anchor destination's resolved path escapes the documentation root -- a lexical check, since the index build side needs none (an escaping key can never be in it) but the query side had no way to tell that guaranteed miss apart from a genuine "not found". A link that escapes root is now Broken: rewriteDocLink reports "LINK BROKEN: ... (outside the documentation root)" and renderLink replaces the whole element -- tags and visible text alike -- with that literal message, the same way images.go already does for a missing image. This needed a small per-node flag on storageRenderer (linkBrokenText) since goldmark still invokes a container node's renderer on the matching leaving call regardless of WalkSkipChildren on entering, and there is no "</a>" to write in that case.
A doc link whose target genuinely doesn't exist under root now reports "LINK BROKEN: ... (not found)" and replaces the published element the same way an escaping or missing image already does, rather than the generic "link not resolved" warning every unresolved link used to get regardless of why. FileExists is what makes this distinguishable from "exists but has no page_id yet", which stays a warning -- that's the normal state of every page in a tree that hasn't been published, not a defect. Updated the two existing regression fixtures whose "unknown" link pointed at a genuinely nonexistent file, and added a new one (link-not-yet-published) so the still-a-warning path -- an existing, unpublished sibling -- has coverage of its own; nothing previously exercised it directly.
Both anchor branches in rewriteHref used to no-op silently on a miss -- a same-page #fragment or a cross-file path.md#fragment that named no real heading published with no word about it at all. Now both warn "anchor not found: ...". The cross-file branch gates on FileExists so a target that doesn't exist at all isn't double-reported: rewriteDocLink already covers that case as Broken on its own. Added a regression fixture covering both branches; nothing existing exercised an anchor miss at all.
nodeLine walks a node to its first descendant *ast.Text (the same Segment substrate nodeText already reads) and converts that byte offset to a 1-indexed line, since neither *ast.Link nor *ast.Image carries its own position -- goldmark's parser never calls SetLines on either. Wired into every existing message site (images.go's four Broken and two Warnings, tables.go's cell-background warning) and the link sites added over the last few commits, via a new storageRenderer.linePrefix helper. lineOffset on storageRenderer corrects for the fact that goldmark parses md.Body, which Extract has already stripped of its frontmatter block -- every position it reports is relative to the body, not the file a reader would open and count lines in. Without it, a document with frontmatter reports a line number that is wrong by exactly the frontmatter's own line count. Regenerated every regression golden this touches (every one of them, since the change lands on pre-existing messages too, not just the ones from the last few commits) and updated the README's example messages to match.
Closes #42. Validates one or more markdown FILEs against the converter and frontmatter rules with no network access and no credentials: it never imports internal/client (guarded by a static import check in check_test.go), never writes to disk, and never contacts Confluence. Frontmatter validation: an unparseable/unterminated block (via frontmatter.Parse's new error), an invalid page_width, a present-but- non-numeric page_id. Deliberately not checked: whether page_id/space/parent are present at all -- check cannot know the caller's intended operation, and a file legitimately has none of these before its first create. Reuses update/create's project.Cache/linkindex.Cache pattern to build the root/index pair MdToConfluence needs, against hardcoded baseURL/spaceKey (the regression suite's own defaults) rather than flags, since neither affects what Broken/Warnings report. --show-html adds the converted storage HTML and attachment list to a result that reached the converter: indented by nesting depth in human output (a per-line indent, not a whitespace-normalizing reformat, since the renderer already breaks lines at every structural boundary) and as a `debug: {html, attachments} | null` field in --json, where html stays compact/unindented to match what update/create would literally publish. Schema: added "check" to the command enum, an if/then branch, and the checkResult/checkSummary/checkAttachment $defs.
The dedicated diagnostic R1's own Partial note named as missing now exists: a doc-link that's missing entirely or escapes the documentation root is Broken and replaces the published element; one that exists but isn't published yet, or whose fragment matches no heading, warns; every message carries its source line; and check adds the "audit without publishing" half with no network access or writes. Scoped explicitly to the two reference kinds markfluence actually attempts to resolve -- doc-links and images. A relative link to a non-.md, non-image local file (e.g. a PDF) is never a resolution attempt at all, so it sits outside R1's claim rather than inside it unmet.
New "Validating markdown locally" usage group, a ### check section (what it checks, the Broken-fails/Warnings-don't rule, --show-html), and updates to the --json section's status-verb and per-target notes plus a new bullet on check's broken-status ok:false-with-no-error/code shape and the debug field.
CLAUDE.md gains a cmd/check/ bullet; the internal/convert writeup gains the link-severity split, the linkBrokenText output-replacement mechanism, and the nodeLine/linePrefix/lineOffset line-number machinery; internal/frontmatter's writeup notes Parse's new error return. Also corrected two pieces of pre-existing drift the same paragraph already carried: MdToConfluence's signature was still shown pre-026 (missing root/index), and links.go's description still referenced a "docKey" that no longer exists and claimed unresolved-link failure is silent, which #42 is what makes untrue.
- docs/guarantees.md: R1's "every message carries the source line" overclaimed relative to nodeLine's own documented ok=false case (an empty link/image alt has no *ast.Text to walk to) -- scoped the wording to match. - frontmatter.go: Parse and Extract each ran frontmatterRE separately; Parse now reuses the one match. Documented and pinned (a new test) the one known false positive this lexical check can't avoid -- a document opening with a bare "---" thematic break and nothing that closes it reads the same as unterminated frontmatter. - cmd/check: checkResult.ok was a stored bool kept in sync with status by hand at four call sites; it's now computed from status, so there's one thing to get right instead of two that could silently disagree. - internal/convert/links.go: the "anchor not found" and "LINK BROKEN: ... (reason)" messages were each built from an identical literal in two separate branches; extracted into warnAnchorNotFound/reportLinkBroken so a future reword can't drift between the two copies.
Neither check --help nor README.md said what the warning's TARGET actually refers to, or named its second, easy-to-misread cause: a same-page anchor (#heading) is internally treated as a link to the current file, so it hits this exact warning too when the current file itself has no page_id yet -- reading as though the file names itself as an unresolved target, when it's just this file before its first publish. Added to both places, and to the "Links to sibling .md files" section's own bullet list.
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
1 participant
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.
Fixes #42.
Summary
markfluence check FILE...: validates markdown against the converter and frontmatter rules with no network access, no credentials, and no writes.checkworth building: a doc-link that's missing entirely or escapes the documentation root is now Broken (replaces the published element, matching how a broken image already behaves); one that exists but isn't published yet, or whose#fragmentmatches no heading, warns instead of publishing silently. EveryBroken/Warningmessage — images, links, table-cell colors — is now prefixed with its source line.frontmatter.Parsenow reports a clear error for an unterminated frontmatter block instead of silently misreading the whole file as body.check --show-htmlprints the converted storage HTML (indented by nesting depth) and attachment list, for debugging what a file would publish without publishing it; the same payload is adebugfield under--json.report-unresolved-references) to Holds indocs/guarantees.md, scoped to the two reference kinds markfluence actually attempts to resolve./code-reviewpass on this branch: fixed a docs overclaim, a redundantok/statusfield, duplicated message templates, and a redundant regex evaluation; filed check warns on a same-page anchor in an unpublished file #118 for a pre-existing (not introduced here) limitationchecknow surfaces more loudly./security-review: no findings.Test plan
make check(vet, fmt-check, test, build, lint) passescmd/checkhas its own end-to-end tests (clean/warnings/broken/failed statuses, multi-root batches,--show-html,--jsonschema conformance) and a static-import guard against ever pulling ininternal/clientmarkfluence checkagainst real fixtures, human and--jsonoutput, including--show-html