Cloonar/deckle

Fork 0

linkrefs docs claim harness check reports dangling links, but no link check exists #195

New issue

Closed

opened 2026-10-10 21:18:04 +02:00 by dominik.polakovics · 2 comments

dominik.polakovics commented

2026-10-10 21:18:04 +02:00

Owner

This was generated by AI during triage.

Summary

harness check reports every internal link whose target does not exist, so the best-effort link rewrite's documented safety net becomes real, and the one rule deciding whether a stored link resolves gets one owner that the check, GET /api/link/resolve and publish's draft-link check all call.

Current behavior

internal/linkrefs/doc.go ("Best-effort, not refusal") and the walkMarkdown comment in internal/linkrefs/linkrefs.go say a link the rewrite skipped is reported by harness check as dangling. internal/checkcmd resolves no link of any shape. Reproduced on a fresh harness init site: a page whose body holds a markdown link to /renamed-away/ and an inline anchor to /about/#c-nouid (the page exists, the uid does not), and whose front matter holds related: /gone/, gets no link finding — only the "not canonical" warning for hand spacing. Since e4d8138 (ADR 0017) nothing records a renamed page's old URL, so a skipped rewrite is a silent 404 on the live site.

Link resolution is evaluated twice today, in the server only: handleLinkResolve (internal/server/linktargets.go) matches the target against the default-language tree's permalinks and looks a #c-<uid> fragment up on the page at any depth (for a non-default language: on the translation file first, then the source); disabledLinkTargets (internal/server/publish.go) matches permalinks and, additionally, a content-path spelling (posts/b/index.md) that predates the stored-link vocabulary and that nothing writes. Resolve reports a bundle sub-path (/news/first/photo.jpg) as found: false, although linkrefs/doc.go and the migrate-site skill ("a link to a bundle file is the page URL plus the filename, never relative") name it as a link shape.

Desired behavior

harness check walks every page and translation file under content/ — draft pages included — and, for every destination linkrefs.Destinations yields that is non-empty, of linkrefs.Page kind and root-relative (starts with /), reports a warning in area content on the file holding the link when the target does not resolve.
A target resolves when its linkrefs.SplitTarget target is a page's permalink in the default-language tree; or that permalink plus a sub-path naming an existing file in that page's bundle directory; and, when the tail carries a #c-<uid> fragment, a shortcode with that uid exists at any depth on the page (for a link in a translation file: on the translation, else on the source — the lookup resolve makes today). Any other fragment, and a query, is display detail and never checked. A draft page is a valid target.
One warning per distinct destination per file. The message names the destination and what is missing: no such page; no such element on that page; no such file in that page's bundle.
Nothing is reported for external URLs, bare #anchors, /media/… links, relative destinations (../about/, about.md), ref/relref shortcodes, empty values, plain text params, or the aliases key. The check produces no false positives: a text param is yielded whole by Destinations and must never warn.
The resolution rule lives in one place, importable by internal/server and internal/checkcmd; handleLinkResolve, disabledLinkTargets and the check call it. Consequently GET /api/link/resolve answers found: true for a bundle sub-path whose file exists, and publish's content-path match is gone. Publish's draft-link warning still fires for a link to a draft page.
The two linkrefs comments name the finding exactly as it exists. docs/harness.md "Check" lists the warning. scaffold/site/AGENTS.md "Verifying a change" and the verify-site skill step 1 name it and say what to do about it (fix the link, or confirm the target was meant to go). The migrate-site skill's "after each section, harness check lists …" sentence includes it. docs/api.md GET /api/link/resolve records the bundle sub-path answer.

Decisions

Settled in the triage grilling on 2026-10-15; implement, do not reopen.

Add the check rather than reword the comments — ADR 0017 removed the alias safety net on the premise that this check exists; without it a skipped rewrite is silent until a visitor hits a 404.
One owner for "does this stored link resolve", called by the check, resolve and publish — CLAUDE.md "Every rule has exactly one owner"; a third evaluation is what a check written inside checkcmd would add. Where it lives (pagetree, which already builds the tree, or a new package above linkrefs and pagetree) is the implementer's call; linkrefs cannot own it, since it must stay below media in the import graph and must not import pagetree.
Targets are page permalinks, #c-<uid> element anchors and bundle sub-paths — the documented stored-link vocabulary. /media/… links are not checked here: no media reference is checked by harness check today, and that check has its own owner (media.ResolvePath); it is #300.
Resolve follows the owner, so bundle sub-paths become found: true — today's found: false for a documented link shape is a defect of its own; a flag that makes only the check accept them would put two behaviours behind one rule.
Publish's content-path match is dropped — the owner accepts exactly what resolve accepts; a content path is a relative destination and nothing writes one. Verified: neither the web client nor the theme stores such a link.
Candidates are filtered to non-empty root-relative Page-kind values before resolution, and relative destinations stay silent — a second rule to tell a relative link from a text param is not worth a false positive; relative destinations are resolved by nothing today.
Draft pages are checked like any page — the check is about the site's files, not the live build; a stale link in a draft goes live the moment the page is enabled.
Warning, not error — the site builds, a stale link is "probably not what was meant" (docs/harness.md's severity rule), and a page whose link cannot be verified must not fail the check any more than it blocks a rename.
One issue — the owner move, the check, its callers and the docs/skills land together, or a second copy of the rule or an undocumented check exists in between.
No ADR — a check is reversible; ADR 0017 stays as written.

Key interfaces

linkrefs.Destinations, linkrefs.Classify, linkrefs.SplitTarget — the one scan and the reading of a candidate; reuse, never add a second scan or regex.
pagetree.Build, pagetree.Node.FindByPermalink, Node.Path (the bundle directory is derivable from it) — the page lookup behind the owner.
checkcmd.Finding, checker.warnf with AreaContent — the finding shape; Counts and the --json shape stay as they are.
handleLinkResolve and disabledLinkTargets — become callers of the owner; linkResolveResponse is unchanged in shape.
Starting hints: internal/checkcmd/check.go (content() already walks and parses every .md file; addresses() already builds the tree per language), internal/server/linktargets.go, internal/server/publish.go, internal/pagetree.

Acceptance criteria

A scaffolded site with the three dangling links from the reproduction gets three content warnings on the linking file, each naming its destination and what is missing; the same site with the links fixed is clean; harness check exits 0 either way.
A link to an existing page, an existing element anchor, an existing bundle file, a draft page, an external URL, a bare #anchor, a /media/… link, a relative destination, an empty link param and a plain text param produce no finding.
A dangling link inside a draft page is reported.
A dangling link in a translation file is reported on the translation file; an element anchor present only on the source page resolves from the translation.
GET /api/link/resolve and the check agree on every case above, including found: true for a bundle sub-path; publish's draft-link warning still fires for a link to a draft page and no longer matches a content-path spelling.
No second implementation of the resolution rule remains in internal/server or internal/checkcmd, and no comment says a function mirrors or must stay in sync with another.
docs/harness.md "Check", scaffold/site/AGENTS.md "Verifying a change", the verify-site and migrate-site skills, docs/api.md and the two linkrefs comments say what the engine now does; the drift tests (docs/docs_test.go, scaffold/skills_test.go, internal/element/agentsdoc_test.go, the route test in internal/server) pass.

How to verify

go test ./internal/checkcmd/ ./internal/linkrefs/ ./internal/server/ ./internal/pagetree/ ./docs/ ./scaffold/ ./internal/element/ and go vet ./... (building ./... needs web/dist; the package tests do not).
The reproduction as a test in internal/checkcmd: scaffoldSite, write content/about/_index.md and a page with the three links above, Run(Options{SiteDir: dir}) — before the change the report holds only the "not canonical" warning, after it holds the three link warnings.
harness check --site <site> on a site after a harness-driven rename whose rewrite skipped a page (an unparsable page linking to the renamed one, fixed by hand afterwards) prints the dangling link.

Assumptions to check

That Destinations already reaches every link position the issue lists (front-matter values at any nesting, block-shortcode children) — read nodeDestinations and deepStrings before relying on it.
That a bundle file is published at the page's permalink plus its filename on multilingual sites too (the stored link is language-neutral; the theme resolves the language) — check ce/href.html and the render-link hook.
That pagetree.Build on the default language is the right tree for a link found in a translation file (resolve assumes so; publish assumes so) and that building it once per check run is enough.

Out of scope

Checking /media/… links or image params against the library — #300.
Resolving relative destinations or ref/relref shortcodes.
A redirect or alias scheme (ADR 0017).
Changing what the link rewrite does on rename or move, or making it refuse.
Any change to the --json shape of harness check beyond the new finding rows.

Blocked by

None - can start immediately

Original report

This was generated by AI while landing a PR.

PR #193 (commit e4d8138) removed every engine-written alias. The link-rewrite package's docs were updated to say that a link the best-effort rewrite skipped is caught later by harness check:

internal/linkrefs/doc.go ("Best-effort, not refusal"): a link that was not rewritten is reported by harness check as dangling rather than left to fail the whole operation.
internal/linkrefs/linkrefs.go, walkMarkdown doc: harness check reports the dangling link.

internal/checkcmd has no link check at all: it never resolves markdown destinations, href values or link-typed params against the page tree. Before #193 the alias a rename recorded was the safety net for a skipped rewrite; now there is none, and the docs point at a check that does not exist.

Desired behavior, in order of preference: add the check (every internal link whose target is no page, element anchor or bundle file of the site, as a warning, with the verify-site skill and AGENTS.md updated), or reword both comments so they do not promise a check.

Related: #193 concern 2 of the land-pr verdict; #194 (no engine-written aliases).

> *This was generated by AI during triage.* ## Summary `harness check` reports every internal link whose target does not exist, so the best-effort link rewrite's documented safety net becomes real, and the one rule deciding whether a stored link resolves gets one owner that the check, `GET /api/link/resolve` and publish's draft-link check all call. ## Current behavior `internal/linkrefs/doc.go` ("Best-effort, not refusal") and the `walkMarkdown` comment in `internal/linkrefs/linkrefs.go` say a link the rewrite skipped is reported by `harness check` as dangling. `internal/checkcmd` resolves no link of any shape. Reproduced on a fresh `harness init` site: a page whose body holds a markdown link to `/renamed-away/` and an inline anchor to `/about/#c-nouid` (the page exists, the uid does not), and whose front matter holds `related: /gone/`, gets no link finding — only the "not canonical" warning for hand spacing. Since `e4d8138` (ADR 0017) nothing records a renamed page's old URL, so a skipped rewrite is a silent 404 on the live site. Link resolution is evaluated twice today, in the server only: `handleLinkResolve` (`internal/server/linktargets.go`) matches the target against the default-language tree's permalinks and looks a `#c-<uid>` fragment up on the page at any depth (for a non-default language: on the translation file first, then the source); `disabledLinkTargets` (`internal/server/publish.go`) matches permalinks and, additionally, a content-path spelling (`posts/b/index.md`) that predates the stored-link vocabulary and that nothing writes. Resolve reports a bundle sub-path (`/news/first/photo.jpg`) as `found: false`, although `linkrefs/doc.go` and the `migrate-site` skill ("a link to a bundle file is the page URL plus the filename, never relative") name it as a link shape. ## Desired behavior - `harness check` walks every page and translation file under `content/` — draft pages included — and, for every destination `linkrefs.Destinations` yields that is non-empty, of `linkrefs.Page` kind and root-relative (starts with `/`), reports a **warning** in area `content` on the file holding the link when the target does not resolve. - A target resolves when its `linkrefs.SplitTarget` target is a page's permalink in the default-language tree; or that permalink plus a sub-path naming an existing file in that page's bundle directory; and, when the tail carries a `#c-<uid>` fragment, a shortcode with that uid exists at any depth on the page (for a link in a translation file: on the translation, else on the source — the lookup resolve makes today). Any other fragment, and a query, is display detail and never checked. A draft page is a valid target. - One warning per distinct destination per file. The message names the destination and what is missing: no such page; no such element on that page; no such file in that page's bundle. - Nothing is reported for external URLs, bare `#anchors`, `/media/…` links, relative destinations (`../about/`, `about.md`), `ref`/`relref` shortcodes, empty values, plain text params, or the `aliases` key. The check produces no false positives: a text param is yielded whole by `Destinations` and must never warn. - The resolution rule lives in one place, importable by `internal/server` and `internal/checkcmd`; `handleLinkResolve`, `disabledLinkTargets` and the check call it. Consequently `GET /api/link/resolve` answers `found: true` for a bundle sub-path whose file exists, and publish's content-path match is gone. Publish's draft-link warning still fires for a link to a draft page. - The two `linkrefs` comments name the finding exactly as it exists. `docs/harness.md` "Check" lists the warning. `scaffold/site/AGENTS.md` "Verifying a change" and the `verify-site` skill step 1 name it and say what to do about it (fix the link, or confirm the target was meant to go). The `migrate-site` skill's "after each section, `harness check` lists …" sentence includes it. `docs/api.md` `GET /api/link/resolve` records the bundle sub-path answer. ## Decisions Settled in the triage grilling on 2026-10-15; implement, do not reopen. - Add the check rather than reword the comments — ADR 0017 removed the alias safety net on the premise that this check exists; without it a skipped rewrite is silent until a visitor hits a 404. - One owner for "does this stored link resolve", called by the check, resolve and publish — CLAUDE.md "Every rule has exactly one owner"; a third evaluation is what a check written inside `checkcmd` would add. Where it lives (`pagetree`, which already builds the tree, or a new package above `linkrefs` and `pagetree`) is the implementer's call; `linkrefs` cannot own it, since it must stay below `media` in the import graph and must not import `pagetree`. - Targets are page permalinks, `#c-<uid>` element anchors and bundle sub-paths — the documented stored-link vocabulary. `/media/…` links are not checked here: no media reference is checked by `harness check` today, and that check has its own owner (`media.ResolvePath`); it is #300. - Resolve follows the owner, so bundle sub-paths become `found: true` — today's `found: false` for a documented link shape is a defect of its own; a flag that makes only the check accept them would put two behaviours behind one rule. - Publish's content-path match is dropped — the owner accepts exactly what resolve accepts; a content path is a relative destination and nothing writes one. Verified: neither the web client nor the theme stores such a link. - Candidates are filtered to non-empty root-relative `Page`-kind values before resolution, and relative destinations stay silent — a second rule to tell a relative link from a text param is not worth a false positive; relative destinations are resolved by nothing today. - Draft pages are checked like any page — the check is about the site's files, not the live build; a stale link in a draft goes live the moment the page is enabled. - Warning, not error — the site builds, a stale link is "probably not what was meant" (`docs/harness.md`'s severity rule), and a page whose link cannot be verified must not fail the check any more than it blocks a rename. - One issue — the owner move, the check, its callers and the docs/skills land together, or a second copy of the rule or an undocumented check exists in between. - No ADR — a check is reversible; ADR 0017 stays as written. ## Key interfaces - `linkrefs.Destinations`, `linkrefs.Classify`, `linkrefs.SplitTarget` — the one scan and the reading of a candidate; reuse, never add a second scan or regex. - `pagetree.Build`, `pagetree.Node.FindByPermalink`, `Node.Path` (the bundle directory is derivable from it) — the page lookup behind the owner. - `checkcmd.Finding`, `checker.warnf` with `AreaContent` — the finding shape; `Counts` and the `--json` shape stay as they are. - `handleLinkResolve` and `disabledLinkTargets` — become callers of the owner; `linkResolveResponse` is unchanged in shape. - Starting hints: `internal/checkcmd/check.go` (`content()` already walks and parses every `.md` file; `addresses()` already builds the tree per language), `internal/server/linktargets.go`, `internal/server/publish.go`, `internal/pagetree`. ## Acceptance criteria - [ ] A scaffolded site with the three dangling links from the reproduction gets three `content` warnings on the linking file, each naming its destination and what is missing; the same site with the links fixed is clean; `harness check` exits 0 either way. - [ ] A link to an existing page, an existing element anchor, an existing bundle file, a draft page, an external URL, a bare `#anchor`, a `/media/…` link, a relative destination, an empty link param and a plain text param produce no finding. - [ ] A dangling link inside a draft page is reported. - [ ] A dangling link in a translation file is reported on the translation file; an element anchor present only on the source page resolves from the translation. - [ ] `GET /api/link/resolve` and the check agree on every case above, including `found: true` for a bundle sub-path; publish's draft-link warning still fires for a link to a draft page and no longer matches a content-path spelling. - [ ] No second implementation of the resolution rule remains in `internal/server` or `internal/checkcmd`, and no comment says a function mirrors or must stay in sync with another. - [ ] `docs/harness.md` "Check", `scaffold/site/AGENTS.md` "Verifying a change", the `verify-site` and `migrate-site` skills, `docs/api.md` and the two `linkrefs` comments say what the engine now does; the drift tests (`docs/docs_test.go`, `scaffold/skills_test.go`, `internal/element/agentsdoc_test.go`, the route test in `internal/server`) pass. ## How to verify - `go test ./internal/checkcmd/ ./internal/linkrefs/ ./internal/server/ ./internal/pagetree/ ./docs/ ./scaffold/ ./internal/element/` and `go vet ./...` (building `./...` needs `web/dist`; the package tests do not). - The reproduction as a test in `internal/checkcmd`: `scaffoldSite`, write `content/about/_index.md` and a page with the three links above, `Run(Options{SiteDir: dir})` — before the change the report holds only the "not canonical" warning, after it holds the three link warnings. - `harness check --site <site>` on a site after a `harness`-driven rename whose rewrite skipped a page (an unparsable page linking to the renamed one, fixed by hand afterwards) prints the dangling link. ## Assumptions to check - That `Destinations` already reaches every link position the issue lists (front-matter values at any nesting, block-shortcode children) — read `nodeDestinations` and `deepStrings` before relying on it. - That a bundle file is published at the page's permalink plus its filename on multilingual sites too (the stored link is language-neutral; the theme resolves the language) — check `ce/href.html` and the render-link hook. - That `pagetree.Build` on the default language is the right tree for a link found in a translation file (resolve assumes so; publish assumes so) and that building it once per check run is enough. ## Out of scope - Checking `/media/…` links or image params against the library — #300. - Resolving relative destinations or `ref`/`relref` shortcodes. - A redirect or alias scheme (ADR 0017). - Changing what the link rewrite does on rename or move, or making it refuse. - Any change to the `--json` shape of `harness check` beyond the new finding rows. ## Blocked by None - can start immediately ## Original report > *This was generated by AI while landing a PR.* PR #193 (commit e4d8138) removed every engine-written alias. The link-rewrite package's docs were updated to say that a link the best-effort rewrite skipped is caught later by `harness check`: - `internal/linkrefs/doc.go` ("Best-effort, not refusal"): *a link that was not rewritten is reported by `harness check` as dangling rather than left to fail the whole operation*. - `internal/linkrefs/linkrefs.go`, `walkMarkdown` doc: *`harness check` reports the dangling link*. `internal/checkcmd` has no link check at all: it never resolves markdown destinations, `href` values or link-typed params against the page tree. Before #193 the alias a rename recorded was the safety net for a skipped rewrite; now there is none, and the docs point at a check that does not exist. Desired behavior, in order of preference: add the check (every internal link whose target is no page, element anchor or bundle file of the site, as a warning, with the `verify-site` skill and AGENTS.md updated), or reword both comments so they do not promise a check. Related: #193 concern 2 of the land-pr verdict; #194 (no engine-written aliases).

dominik.polakovics added the

needs-triage

label

2026-10-10 21:18:04 +02:00

dominik.polakovics commented

2026-10-15 10:57:54 +02:00

Author

Owner

This was generated by AI during triage.

Triage Notes

What we've established so far:

Reproduced. On a fresh harness init site, a page whose body links to /renamed-away/ (markdown) and /about/#c-nouid (inline <a href>, page exists, uid does not) and whose front matter holds related: /gone/ gets one finding from harness check: the "not canonical" warning for my hand spacing. No link finding. internal/checkcmd/check.go contains no link code at all; the only "dangling" it knows is sidecar names.
Both comments were written in e4d8138 (PR #193), the commit that removed every engine-written alias. ADR 0017 records that decision; its "Consequences" lean on the same non-existent safety net. So the docs are not stale — they promised a check that was never built.
The rule already has an owner, just not one checkcmd can call. "Does this stored link resolve" is evaluated today by GET /api/link/resolve (handleLinkResolve in internal/server/linktargets.go: linkrefs.Classify → linkrefs.SplitTarget → FindByPermalink on the default-language page tree → #c-<uid> shortcode lookup at any depth), and a second time by publish's draft-link check (disabledLinkTargets in internal/server/publish.go, which also accepts a content-path spelling such as posts/b/index.md). A third evaluation inside checkcmd is exactly what CLAUDE.md "Every rule has exactly one owner" forbids: adding the check means moving the resolution rule to a package the server and checkcmd both import and making all three callers use it. linkrefs imports only content, pagetree does not import linkrefs, so the owner can live in pagetree or in a new package above both; that choice is the agent's.
linkrefs.Destinations yields candidates, not links: every string shortcode param and every front-matter string is yielded whole, so a plain text param ("Hello world") and an empty optional link param both come out as Page kind. Publish gets away with it by looking candidates up in a map of draft targets; a dangling check must filter first, or every text param becomes a warning. Relative destinations and ref/relref shortcodes are not resolved by anything today (publish documents that as its limitation).
Bundle sub-paths are a documented link shape (linkrefs/doc.go, and the migrate-site skill: "a link to a bundle file is the page URL plus the filename, never relative"), but /api/link/resolve reports them found: false today because the match is the exact permalink. Whatever owner the check calls has to accept them, and then resolve does too.
Nothing in .out-of-scope/ (the directory does not exist).
Documents and skills the fix touches, if the check is added: docs/harness.md "Check" (the warnings list), scaffold/site/AGENTS.md "Verifying a change" (the paragraph listing what the check reports), the verify-site skill step 1 (same list, and what to do with the new warning), the migrate-site skill (the "after each section, harness check lists …" sentence, where a dangling-link warning is most useful), docs/api.md GET /api/link/resolve if bundle sub-paths become found: true, and the two linkrefs comments so they name the finding precisely. No ADR: a check is not hard to reverse. If only the docs are fixed, just the two comments and one sentence in ADR 0017's consequences.

Recommendation: bug, and option 1 (add the check). ADR 0017 removed the alias safety net on the premise that this check exists; without it a rewrite the best-effort pass skips is silent until a visitor hits a 404 on the live site. The resolution rule exists already, so the work is relocating it and walking content/ with it, not designing anything new.

Decisions for you (a short reply settles them; my recommended answer first):

Add the check, or fix the docs only? Recommend add. If you prefer docs-only, this becomes a two-comment change plus a sentence in ADR 0017, and I will write that brief instead.
Which targets count? Recommend: page permalinks, #c-<uid> element anchors and bundle sub-paths — the issue's list, which is linkrefs' documented vocabulary. /media/… links stay out: neither image params nor media links are checked by harness check today, so a media check is its own issue, not a side effect of this one.
Bundle sub-paths become found: true in /api/link/resolve too. Recommend yes: one owner means resolve inherits the rule, and today's found: false for a documented link shape is a defect of its own. The alternative is to strike bundle sub-paths from the link shapes in linkrefs/doc.go and migrate-site.
Publish's content-path spelling. When the rule moves to one owner, the owner accepts what resolve accepts (permalinks). Recommend dropping the content-path match (posts/b/index.md) from publish's draft check rather than teaching the owner a spelling nothing writes and resolve reports as dangling. Commit 68857e1 added it without a stated reason; the agent should confirm there is none before dropping it.
What is reported. Recommend: a warning, area content, on the file holding the link (a translation file for a translation), one per distinct destination per file, for every non-empty root-relative destination of Page kind whose target is no page, whose #c-<uid> names no element on that page, or whose sub-path is no file of that page's bundle. Relative destinations, bare #anchors, external URLs and empty values draw nothing — no false positives, as the best-effort contract already promises. A link to a draft page is not dangling (publish warns about those).
Slicing. Recommend one issue: the owner move, the check and the docs/skills land together, or the "one owner" rule is broken in between.

Draft brief (option 1, with the recommended answers above)

Replaces the issue body once the decisions are confirmed.

Summary

harness check reports every internal link whose target does not exist, so the best-effort link rewrite's documented safety net is real, and the one rule deciding whether a stored link resolves has one owner that the check, /api/link/resolve and publish's draft-link check all call.

Current behavior

internal/linkrefs/doc.go ("Best-effort, not refusal") and the walkMarkdown comment in internal/linkrefs/linkrefs.go say a link the rewrite skipped is reported by harness check as dangling. internal/checkcmd resolves no link of any shape. Reproduced on a scaffolded site: a markdown link to /renamed-away/, an inline anchor to /about/#c-nouid and a front-matter value /gone/ produce no finding. Since e4d8138 (ADR 0017) nothing else records a renamed page's old URL, so a skipped rewrite is a silent 404 on the live site.

Link resolution is evaluated twice today: handleLinkResolve (internal/server/linktargets.go) matches the target against the default-language tree's permalinks and looks the #c-<uid> up on the page; disabledLinkTargets (internal/server/publish.go) matches permalinks and, additionally, a content-path spelling. Resolve reports a bundle sub-path (/news/first/photo.jpg) as not found although linkrefs/doc.go and the migrate-site skill name it as a link shape.

Desired behavior

harness check walks every page and translation file under content/ and, for every destination linkrefs.Destinations yields that is non-empty, of linkrefs.Page kind and root-relative (starts with /), reports a warning in area content on the file holding the link when the target does not resolve. A target resolves when its SplitTarget target is a page's permalink in the default-language tree; or that permalink plus a sub-path naming an existing file in that page's bundle directory; and, when the tail carries a #c-<uid> fragment, a shortcode with that uid exists at any depth on the page (for a link in a translation file: on the translation, else on the source, as resolve does today). Any other fragment or a query is display detail and never checked.
One warning per distinct destination per file; the message names the destination and what is missing (no such page / no such element on that page / no such file in that page's bundle). A draft page is a valid target.
Nothing is reported for external URLs, bare #anchors, /media/… links, relative destinations, ref/relref shortcodes, empty values or the aliases key — the check must not produce false positives.
The resolution rule lives in one place, importable by internal/server and internal/checkcmd, and handleLinkResolve, disabledLinkTargets and the check call it. /api/link/resolve therefore answers found: true for a bundle sub-path whose file exists. Publish's content-path match is dropped unless the agent finds a stated reason for it.
The two linkrefs comments name the finding exactly as it exists; docs/harness.md "Check" lists the warning; scaffold/site/AGENTS.md "Verifying a change" and the verify-site skill name it and say what to do about it (fix the link, or confirm the target was meant to go); the migrate-site skill's post-section check sentence lists it; docs/api.md GET /api/link/resolve records the bundle sub-path answer.

Decisions

One owner for "does this stored link resolve", called by the check, resolve and publish — CLAUDE.md "Every rule has exactly one owner"; a third evaluation is what the issue would otherwise add. Where it lives (pagetree, which already builds the tree, or a new package above linkrefs and pagetree) is the agent's call; linkrefs cannot own it because it must stay below media and must not import pagetree.
Warning, not error — the site builds; a stale link is "probably not what was meant" (docs/harness.md's severity rule), and a page whose link cannot be verified must not block harness check any more than it blocks a rename.
Candidates are filtered to non-empty root-relative Page-kind values before resolution — Destinations yields every string param whole; without the filter every text param is a finding.
Bundle sub-paths resolve — they are a documented link shape; the alternative was to un-document them.
/media/… links are not checked here — no media reference is checked by harness check today; that is a separate check with its own owner (media.ResolvePath).
No ADR — a check is reversible; ADR 0017 stays as written.

Key interfaces

linkrefs.Destinations, linkrefs.Classify, linkrefs.SplitTarget — the scan and the reading of a candidate; reuse, do not add a second scan.
pagetree.Node.FindByPermalink, pagetree.Build — the page lookup; the bundle directory of a node is derivable from Node.Path.
checkcmd.Finding / checker.warnf with AreaContent — the finding shape; Counts stays as is.
handleLinkResolve and disabledLinkTargets — become callers of the owner.
Starting hints: internal/checkcmd/check.go (content() already walks every .md file and parses it; addresses() already builds the tree per language), internal/server/linktargets.go, internal/server/publish.go.

Acceptance criteria

A scaffolded site with the three dangling links above gets three content warnings on the linking file, each naming the destination; the same site with the links fixed is clean.
A link to an existing page, to an existing element anchor, to an existing bundle file, to a draft page, an external URL, a bare #anchor, a /media/… link, a relative destination, an empty link param and a plain text param produce no finding.
A dangling link in a translation file is reported on the translation file; an element anchor present only on the source page resolves from the translation.
The same site's /api/link/resolve and the check agree on every case above, including found: true for a bundle sub-path; publish's draft-link warning still fires for a link to a draft page.
No second implementation of the resolution rule remains in internal/server or internal/checkcmd (no "same as" comment either).
docs/harness.md "Check", scaffold/site/AGENTS.md "Verifying a change", verify-site and migrate-site skills, docs/api.md and the two linkrefs comments say what the engine now does; the drift tests (docs/docs_test.go, scaffold/skills_test.go, the route test in internal/server) pass.

How to verify

go test ./internal/checkcmd/ ./internal/linkrefs/ ./internal/server/ ./internal/pagetree/ ./docs/ ./scaffold/ and go vet ./...
The repro: harness check --site <scaffolded site> with the three links above; before the change it prints only "not canonical", after it prints three link warnings and exits 0.

Assumptions to check

That Destinations already reaches every link position the issue lists (front-matter values at any nesting, block-shortcode children) — read nodeDestinations/deepStrings before relying on it.
That nothing writes a content-path link (posts/b/index.md) — grep the web client and the themes before dropping publish's match.
That a bundle file is published at the page's permalink plus its filename on multilingual sites too (the stored link is language-neutral; the theme resolves the language).

Out of scope

Checking /media/… links or image params against the library (a separate issue).
Resolving relative destinations or ref/relref shortcodes.
A redirect or alias scheme (ADR 0017).
Changing what the link rewrite does on rename or move.

Blocked by

None - can start immediately

> *This was generated by AI during triage.* ## Triage Notes **What we've established so far:** - **Reproduced.** On a fresh `harness init` site, a page whose body links to `/renamed-away/` (markdown) and `/about/#c-nouid` (inline `<a href>`, page exists, uid does not) and whose front matter holds `related: /gone/` gets one finding from `harness check`: the "not canonical" warning for my hand spacing. No link finding. `internal/checkcmd/check.go` contains no link code at all; the only "dangling" it knows is sidecar names. - Both comments were written in `e4d8138` (PR #193), the commit that removed every engine-written alias. ADR 0017 records that decision; its "Consequences" lean on the same non-existent safety net. So the docs are not stale — they promised a check that was never built. - **The rule already has an owner, just not one `checkcmd` can call.** "Does this stored link resolve" is evaluated today by `GET /api/link/resolve` (`handleLinkResolve` in `internal/server/linktargets.go`: `linkrefs.Classify` → `linkrefs.SplitTarget` → `FindByPermalink` on the default-language page tree → `#c-<uid>` shortcode lookup at any depth), and a second time by publish's draft-link check (`disabledLinkTargets` in `internal/server/publish.go`, which also accepts a content-path spelling such as `posts/b/index.md`). A third evaluation inside `checkcmd` is exactly what CLAUDE.md "Every rule has exactly one owner" forbids: adding the check means moving the resolution rule to a package the server and `checkcmd` both import and making all three callers use it. `linkrefs` imports only `content`, `pagetree` does not import `linkrefs`, so the owner can live in `pagetree` or in a new package above both; that choice is the agent's. - `linkrefs.Destinations` yields *candidates*, not links: every string shortcode param and every front-matter string is yielded whole, so a plain text param ("Hello world") and an empty optional link param both come out as `Page` kind. Publish gets away with it by looking candidates up in a map of draft targets; a dangling check must filter first, or every text param becomes a warning. Relative destinations and `ref`/`relref` shortcodes are not resolved by anything today (publish documents that as its limitation). - **Bundle sub-paths are a documented link shape** (`linkrefs/doc.go`, and the `migrate-site` skill: "a link to a bundle file is the page URL plus the filename, never relative"), but `/api/link/resolve` reports them `found: false` today because the match is the exact permalink. Whatever owner the check calls has to accept them, and then resolve does too. - Nothing in `.out-of-scope/` (the directory does not exist). - Documents and skills the fix touches, if the check is added: `docs/harness.md` "Check" (the warnings list), `scaffold/site/AGENTS.md` "Verifying a change" (the paragraph listing what the check reports), the `verify-site` skill step 1 (same list, and what to do with the new warning), the `migrate-site` skill (the "after each section, `harness check` lists …" sentence, where a dangling-link warning is most useful), `docs/api.md` `GET /api/link/resolve` if bundle sub-paths become `found: true`, and the two `linkrefs` comments so they name the finding precisely. No ADR: a check is not hard to reverse. If only the docs are fixed, just the two comments and one sentence in ADR 0017's consequences. **Recommendation:** `bug`, and option 1 (add the check). ADR 0017 removed the alias safety net on the premise that this check exists; without it a rewrite the best-effort pass skips is silent until a visitor hits a 404 on the live site. The resolution rule exists already, so the work is relocating it and walking `content/` with it, not designing anything new. **Decisions for you (a short reply settles them; my recommended answer first):** 1. **Add the check, or fix the docs only?** Recommend add. If you prefer docs-only, this becomes a two-comment change plus a sentence in ADR 0017, and I will write that brief instead. 2. **Which targets count?** Recommend: page permalinks, `#c-<uid>` element anchors and bundle sub-paths — the issue's list, which is `linkrefs`' documented vocabulary. `/media/…` links stay out: neither image params nor media links are checked by `harness check` today, so a media check is its own issue, not a side effect of this one. 3. **Bundle sub-paths become `found: true` in `/api/link/resolve` too.** Recommend yes: one owner means resolve inherits the rule, and today's `found: false` for a documented link shape is a defect of its own. The alternative is to strike bundle sub-paths from the link shapes in `linkrefs/doc.go` and `migrate-site`. 4. **Publish's content-path spelling.** When the rule moves to one owner, the owner accepts what resolve accepts (permalinks). Recommend dropping the content-path match (`posts/b/index.md`) from publish's draft check rather than teaching the owner a spelling nothing writes and resolve reports as dangling. Commit `68857e1` added it without a stated reason; the agent should confirm there is none before dropping it. 5. **What is reported.** Recommend: a `warning`, area `content`, on the file holding the link (a translation file for a translation), one per distinct destination per file, for every non-empty root-relative destination of `Page` kind whose target is no page, whose `#c-<uid>` names no element on that page, or whose sub-path is no file of that page's bundle. Relative destinations, bare `#anchors`, external URLs and empty values draw nothing — no false positives, as the best-effort contract already promises. A link to a draft page is not dangling (publish warns about those). 6. **Slicing.** Recommend one issue: the owner move, the check and the docs/skills land together, or the "one owner" rule is broken in between. --- ## Draft brief (option 1, with the recommended answers above) Replaces the issue body once the decisions are confirmed. ### Summary `harness check` reports every internal link whose target does not exist, so the best-effort link rewrite's documented safety net is real, and the one rule deciding whether a stored link resolves has one owner that the check, `/api/link/resolve` and publish's draft-link check all call. ### Current behavior `internal/linkrefs/doc.go` ("Best-effort, not refusal") and the `walkMarkdown` comment in `internal/linkrefs/linkrefs.go` say a link the rewrite skipped is reported by `harness check` as dangling. `internal/checkcmd` resolves no link of any shape. Reproduced on a scaffolded site: a markdown link to `/renamed-away/`, an inline anchor to `/about/#c-nouid` and a front-matter value `/gone/` produce no finding. Since `e4d8138` (ADR 0017) nothing else records a renamed page's old URL, so a skipped rewrite is a silent 404 on the live site. Link resolution is evaluated twice today: `handleLinkResolve` (`internal/server/linktargets.go`) matches the target against the default-language tree's permalinks and looks the `#c-<uid>` up on the page; `disabledLinkTargets` (`internal/server/publish.go`) matches permalinks and, additionally, a content-path spelling. Resolve reports a bundle sub-path (`/news/first/photo.jpg`) as not found although `linkrefs/doc.go` and the `migrate-site` skill name it as a link shape. ### Desired behavior - `harness check` walks every page and translation file under `content/` and, for every destination `linkrefs.Destinations` yields that is non-empty, of `linkrefs.Page` kind and root-relative (starts with `/`), reports a **warning** in area `content` on the file holding the link when the target does not resolve. A target resolves when its `SplitTarget` target is a page's permalink in the default-language tree; or that permalink plus a sub-path naming an existing file in that page's bundle directory; and, when the tail carries a `#c-<uid>` fragment, a shortcode with that uid exists at any depth on the page (for a link in a translation file: on the translation, else on the source, as resolve does today). Any other fragment or a query is display detail and never checked. - One warning per distinct destination per file; the message names the destination and what is missing (no such page / no such element on that page / no such file in that page's bundle). A draft page is a valid target. - Nothing is reported for external URLs, bare `#anchors`, `/media/…` links, relative destinations, `ref`/`relref` shortcodes, empty values or the `aliases` key — the check must not produce false positives. - The resolution rule lives in one place, importable by `internal/server` and `internal/checkcmd`, and `handleLinkResolve`, `disabledLinkTargets` and the check call it. `/api/link/resolve` therefore answers `found: true` for a bundle sub-path whose file exists. Publish's content-path match is dropped unless the agent finds a stated reason for it. - The two `linkrefs` comments name the finding exactly as it exists; `docs/harness.md` "Check" lists the warning; `scaffold/site/AGENTS.md` "Verifying a change" and the `verify-site` skill name it and say what to do about it (fix the link, or confirm the target was meant to go); the `migrate-site` skill's post-section check sentence lists it; `docs/api.md` `GET /api/link/resolve` records the bundle sub-path answer. ### Decisions - One owner for "does this stored link resolve", called by the check, resolve and publish — CLAUDE.md "Every rule has exactly one owner"; a third evaluation is what the issue would otherwise add. Where it lives (`pagetree`, which already builds the tree, or a new package above `linkrefs` and `pagetree`) is the agent's call; `linkrefs` cannot own it because it must stay below `media` and must not import `pagetree`. - Warning, not error — the site builds; a stale link is "probably not what was meant" (`docs/harness.md`'s severity rule), and a page whose link cannot be verified must not block `harness check` any more than it blocks a rename. - Candidates are filtered to non-empty root-relative `Page`-kind values before resolution — `Destinations` yields every string param whole; without the filter every text param is a finding. - Bundle sub-paths resolve — they are a documented link shape; the alternative was to un-document them. - `/media/…` links are not checked here — no media reference is checked by `harness check` today; that is a separate check with its own owner (`media.ResolvePath`). - No ADR — a check is reversible; ADR 0017 stays as written. ### Key interfaces - `linkrefs.Destinations`, `linkrefs.Classify`, `linkrefs.SplitTarget` — the scan and the reading of a candidate; reuse, do not add a second scan. - `pagetree.Node.FindByPermalink`, `pagetree.Build` — the page lookup; the bundle directory of a node is derivable from `Node.Path`. - `checkcmd.Finding` / `checker.warnf` with `AreaContent` — the finding shape; `Counts` stays as is. - `handleLinkResolve` and `disabledLinkTargets` — become callers of the owner. - Starting hints: `internal/checkcmd/check.go` (`content()` already walks every `.md` file and parses it; `addresses()` already builds the tree per language), `internal/server/linktargets.go`, `internal/server/publish.go`. ### Acceptance criteria - [ ] A scaffolded site with the three dangling links above gets three `content` warnings on the linking file, each naming the destination; the same site with the links fixed is clean. - [ ] A link to an existing page, to an existing element anchor, to an existing bundle file, to a draft page, an external URL, a bare `#anchor`, a `/media/…` link, a relative destination, an empty link param and a plain text param produce no finding. - [ ] A dangling link in a translation file is reported on the translation file; an element anchor present only on the source page resolves from the translation. - [ ] The same site's `/api/link/resolve` and the check agree on every case above, including `found: true` for a bundle sub-path; publish's draft-link warning still fires for a link to a draft page. - [ ] No second implementation of the resolution rule remains in `internal/server` or `internal/checkcmd` (no "same as" comment either). - [ ] `docs/harness.md` "Check", `scaffold/site/AGENTS.md` "Verifying a change", `verify-site` and `migrate-site` skills, `docs/api.md` and the two `linkrefs` comments say what the engine now does; the drift tests (`docs/docs_test.go`, `scaffold/skills_test.go`, the route test in `internal/server`) pass. ### How to verify - `go test ./internal/checkcmd/ ./internal/linkrefs/ ./internal/server/ ./internal/pagetree/ ./docs/ ./scaffold/` and `go vet ./...` - The repro: `harness check --site <scaffolded site>` with the three links above; before the change it prints only "not canonical", after it prints three link warnings and exits 0. ### Assumptions to check - That `Destinations` already reaches every link position the issue lists (front-matter values at any nesting, block-shortcode children) — read `nodeDestinations`/`deepStrings` before relying on it. - That nothing writes a content-path link (`posts/b/index.md`) — grep the web client and the themes before dropping publish's match. - That a bundle file is published at the page's permalink plus its filename on multilingual sites too (the stored link is language-neutral; the theme resolves the language). ### Out of scope - Checking `/media/…` links or image params against the library (a separate issue). - Resolving relative destinations or `ref`/`relref` shortcodes. - A redirect or alias scheme (ADR 0017). - Changing what the link rewrite does on rename or move. ### Blocked by None - can start immediately

dominik.polakovics added the

bug

label

2026-10-15 10:57:55 +02:00

dominik.polakovics referenced this issue

2026-10-15 11:06:34 +02:00

harness check: report media references (image params, /media/ links) that resolve to no file #300

dominik.polakovics added

ready-for-agent

and removed

needs-triage

labels

2026-10-15 11:07:24 +02:00

dominik.polakovics commented

2026-10-15 11:07:24 +02:00

Author

Owner

This was generated by AI during triage.

Grilled with the maintainer on 2026-10-15: every decision in the triage notes above landed on the recommended answer (add the check; pages, element anchors and bundle sub-paths; resolve follows the owner; drop publish's content-path match; relative destinations silent; draft pages checked; warning; one issue). The body is now the agent brief. Media references are a separate check: #300.

> *This was generated by AI during triage.* Grilled with the maintainer on 2026-10-15: every decision in the triage notes above landed on the recommended answer (add the check; pages, element anchors and bundle sub-paths; resolve follows the owner; drop publish's content-path match; relative destinations silent; draft pages checked; warning; one issue). The body is now the agent brief. Media references are a separate check: #300.

dominik.polakovics referenced this issue from a pull request that will close it,

2026-10-15 11:24:48 +02:00

feat(check): warn on internal links that do not resolve #301

dominik.polakovics referenced this issue

2026-10-15 17:17:43 +02:00

feat(check): warn on internal links that do not resolve #301

dominik.polakovics closed this issue

2026-10-15 17:19:44 +02:00