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

Merged

dominik.polakovics merged 4 commits from afk/195 into main 2026-10-15 17:19:44 +02:00

Conversation 4 Commits 4 Files changed 31 +949 -263

dominik.polakovics commented 2026-10-15 11:24:48 +02:00

Owner

Copy link

Closes #195

What

• One owner for "does this stored link resolve": new internal/linkresolve. It covers page permalinks, #c-<uid> element anchors (translation file first, then the source) and bundle sub-paths, and returns a verdict: resolved, no page, no element or no file. It is called by:

  • harness check
  • GET /api/link/resolve (handleLinkResolve)
  • publish's draft-link check (disabledLinkTargets)

  No second copy of the rule is left in internal/server or internal/checkcmd. ElementAnchorPrefix moved there too.

• harness check warns in area content, on the linking file, once per distinct destination. It checks every non-empty, root-relative, Page-kind destination that linkrefs.Destinations yields from pages and translation files, drafts included. The three messages:

  • links to <dest>, but no page has that address
  • links to <dest>, but <permalink> has no element with uid "<uid>"
  • links to <dest>, but the bundle of <permalink> has no file "<sub>"

• GET /api/link/resolve now answers found: true (kind page, label the file name, no address or fallback) for a bundle file that exists.

• Publish no longer matches a content-path spelling such as posts/b/index.md. A permalink, element or bundle-file link to a draft page still warns.

• linkrefs.Destinations now also skips the top-level front-matter url key, which is the page's own address. Without this, every translation with a url (/kontakt/ in testdata/multilingual-site) and every url without a trailing slash got a false warning. The rename rewrite of url is unchanged.

• Docs: docs/harness.md "Check", docs/api.md (resolve, publish, page refs), scaffold/site/AGENTS.md "Verifying a change", the verify-site and migrate-site skills, and both linkrefs comments.

Decisions within the brief

• A branch bundle's (_index.md) files count only directly in its directory; a leaf bundle's (index.md) count at any depth. This is how Hugo publishes them.
• A sub-path with no extension, or ending in .md, is reported as "no page", so /gone and /posts/b/index.md don't claim a missing bundle file.

Verification

• go test ./... and go vet ./... pass (with web/dist built), and gofmt is clean.
• Reproduction on a fresh harness init site: exactly the three link warnings, exit 0.
• New tests:
  • internal/linkresolve: unit tests for the rule.
  • internal/checkcmd: TestDanglingLinks (the issue's reproduction, then fixed), TestLinksWithoutFalsePositives and TestTranslationLinks.
  • internal/server: bundle file found and missing in resolve, and TestPublishAllCheckIgnoresContentPaths.

Worth knowing (not changed here)

• testdata/library-site now warns on page-teasers section="/posts". The page-teasers sidecar says the theme still resolves that legacy spelling through GetPage, but resolve already reported it found: false. The owner follows the issue: an exact permalink only.
• testdata/site warns on image="/images/hero.png". The file really does not exist.
• Files under static/ would draw a "no file" warning. No scaffold ships static/, but a migrated site might, so this may be worth a follow-up.

🤖 Generated with Claude Code

Closes #195 ## What - **One owner for "does this stored link resolve"**: new `internal/linkresolve`. It covers page permalinks, `#c-<uid>` element anchors (translation file first, then the source) and bundle sub-paths, and returns a verdict: resolved, no page, no element or no file. It is called by: - `harness check` - `GET /api/link/resolve` (`handleLinkResolve`) - publish's draft-link check (`disabledLinkTargets`) No second copy of the rule is left in `internal/server` or `internal/checkcmd`. `ElementAnchorPrefix` moved there too. - **`harness check`** warns in area `content`, on the linking file, once per distinct destination. It checks every non-empty, root-relative, `Page`-kind destination that `linkrefs.Destinations` yields from pages and translation files, drafts included. The three messages: - `links to <dest>, but no page has that address` - `links to <dest>, but <permalink> has no element with uid "<uid>"` - `links to <dest>, but the bundle of <permalink> has no file "<sub>"` - **`GET /api/link/resolve`** now answers `found: true` (kind `page`, label the file name, no address or fallback) for a bundle file that exists. - **Publish** no longer matches a content-path spelling such as `posts/b/index.md`. A permalink, element or bundle-file link to a draft page still warns. - **`linkrefs.Destinations`** now also skips the top-level front-matter `url` key, which is the page's own address. Without this, every translation with a `url` (`/kontakt/` in `testdata/multilingual-site`) and every `url` without a trailing slash got a false warning. The rename rewrite of `url` is unchanged. - **Docs**: `docs/harness.md` "Check", `docs/api.md` (resolve, publish, page refs), `scaffold/site/AGENTS.md` "Verifying a change", the `verify-site` and `migrate-site` skills, and both `linkrefs` comments. ## Decisions within the brief - A branch bundle's (`_index.md`) files count only directly in its directory; a leaf bundle's (`index.md`) count at any depth. This is how Hugo publishes them. - A sub-path with no extension, or ending in `.md`, is reported as "no page", so `/gone` and `/posts/b/index.md` don't claim a missing bundle file. ## Verification - `go test ./...` and `go vet ./...` pass (with `web/dist` built), and `gofmt` is clean. - Reproduction on a fresh `harness init` site: exactly the three link warnings, exit 0. - New tests: - `internal/linkresolve`: unit tests for the rule. - `internal/checkcmd`: `TestDanglingLinks` (the issue's reproduction, then fixed), `TestLinksWithoutFalsePositives` and `TestTranslationLinks`. - `internal/server`: bundle file found and missing in resolve, and `TestPublishAllCheckIgnoresContentPaths`. ## Worth knowing (not changed here) - **`testdata/library-site`** now warns on `page-teasers section="/posts"`. The page-teasers sidecar says the theme still resolves that legacy spelling through `GetPage`, but resolve already reported it `found: false`. The owner follows the issue: an exact permalink only. - **`testdata/site`** warns on `image="/images/hero.png"`. The file really does not exist. - **Files under `static/`** would draw a "no file" warning. No scaffold ships `static/`, but a migrated site might, so this may be worth a follow-up. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

dominik.polakovics added 2 commits 2026-10-15 11:24:48 +02:00

feat(check): warn on internal links that do not resolve ... 79328e9281

harness check now resolves every root-relative page link of every page
and translation file, drafts included, and warns on the linking file when
the target is no page, the #c-<uid> names no element on it, or the
sub-path names no file of its bundle. This is the safety net the
best-effort rename rewrite's docs promised since ADR 0017.

The rule has one owner, internal/linkresolve, which the check,
GET /api/link/resolve and publish's draft-link check all call. Resolve
therefore answers found: true for an existing bundle file, and publish
no longer matches the content-path spelling nothing writes.

linkrefs.Destinations skips the front-matter url key: it is the page's
own address, not a link, and read as one it warned on every translation
with a url and every url without a trailing slash.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

docs: document the dangling-link warning of harness check ... de672069ce

docs/harness.md "Check", AGENTS.md "Verifying a change", the verify-site
and migrate-site skills name the warning and what to do about it;
docs/api.md records the bundle-file answer of GET /api/link/resolve and
drops publish's content-path match.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

dominik.polakovics commented 2026-10-15 11:31:44 +02:00

Author

Owner

Copy link

[autoland] verdict: pass

Verdict: CONCERNS (non-blocking): approved, not auto-merged.

What I relied on: labctl pr checks 301 came back success. ci / build-and-test ran the frontend build and tests, go vet ./..., go test ./... (the drift tests included) and the harness build. I also ran gofmt -l on the changed Go files, which was clean. The head already contains origin/main, so there is nothing to merge in. The title is in Conventional Commits form and the body has Closes #195. The diff matches the brief:

• The rule has one owner, internal/linkresolve, and resolve, publish and harness check all call it. No copy is left in internal/server or internal/checkcmd.
• The tests cover every acceptance criterion.
• docs/harness.md, docs/api.md, AGENTS.md, the verify-site and migrate-site skills, and both linkrefs comments are updated.
• Skipping the url key in linkrefs.Destinations (internal/linkrefs/linkrefs.go:387) is needed to avoid false positives, and the matching /api/page/refs change is documented in docs/api.md.

Concern: harness check warns on a page value the engine accepts and the theme renders

• internal/element/ftype_page.go:22-30 (TypePage) deliberately accepts a page param missing only its trailing slash (/posts for /posts/). The reason given there: params upgraded from text, namely the scaffolded page-teasers section, "left exactly that hand-typed shape in existing content, and Hugo's site.GetPage resolves it at render time".
• scaffold/editor/elements/page-teasers.yaml:3-5 promises the same ("legacy hand-typed paths like "/posts" still resolve").
• linkresolve.Resolver.Resolve (internal/linkresolve/linkresolve.go:147) matches the exact permalink only. So checkcmd.links (internal/checkcmd/check.go:600) reports such a value as links to /posts, but no page has that address.
• Reproduced: running checkcmd.Run on testdata/library-site gives warning content/_index.md: links to /posts, but no page has that address for page-teasers section="/posts" (testdata/library-site/content/_index.md:46). The save path accepts that value and the theme renders it.
• The PR follows the brief's "exact permalink" rule as written, and /api/link/resolve already answered found: false for this value. Still, the engine now gives two different answers to "does this value name an existing page": element.pageKind.site allows the slash, linkresolve does not. The brief also says the check produces no false positives.
• The fix suggested by the warning, writing /posts/, is harmless. The maintainer should decide whether the owner adopts the trailing-slash allowance for page-typed values, or the warning stands and the legacy shape is migrated.

Minor, related (already disclosed in the PR body): a link to a file under static/ (e.g. /images/x.png) gets the misleading the bundle of / has no file "images/x.png" (internal/linkresolve/linkresolve.go:184). No scaffold ships static/, but a migrated site could, so this is worth a follow-up issue.

[autoland] verdict: pass **Verdict: CONCERNS (non-blocking)**: approved, not auto-merged. **What I relied on:** `labctl pr checks 301` came back `success`. `ci / build-and-test` ran the frontend build and tests, `go vet ./...`, `go test ./...` (the drift tests included) and the `harness` build. I also ran `gofmt -l` on the changed Go files, which was clean. The head already contains `origin/main`, so there is nothing to merge in. The title is in Conventional Commits form and the body has `Closes #195`. The diff matches the brief: - The rule has one owner, `internal/linkresolve`, and resolve, publish and `harness check` all call it. No copy is left in `internal/server` or `internal/checkcmd`. - The tests cover every acceptance criterion. - `docs/harness.md`, `docs/api.md`, `AGENTS.md`, the `verify-site` and `migrate-site` skills, and both `linkrefs` comments are updated. - Skipping the `url` key in `linkrefs.Destinations` (`internal/linkrefs/linkrefs.go:387`) is needed to avoid false positives, and the matching `/api/page/refs` change is documented in `docs/api.md`. **Concern: `harness check` warns on a page value the engine accepts and the theme renders** - `internal/element/ftype_page.go:22-30` (`TypePage`) deliberately accepts a `page` param missing only its trailing slash (`/posts` for `/posts/`). The reason given there: params upgraded from text, namely the scaffolded page-teasers `section`, "left exactly that hand-typed shape in existing content, and Hugo's `site.GetPage` resolves it at render time". - `scaffold/editor/elements/page-teasers.yaml:3-5` promises the same ("legacy hand-typed paths like "/posts" still resolve"). - `linkresolve.Resolver.Resolve` (`internal/linkresolve/linkresolve.go:147`) matches the exact permalink only. So `checkcmd.links` (`internal/checkcmd/check.go:600`) reports such a value as `links to /posts, but no page has that address`. - Reproduced: running `checkcmd.Run` on `testdata/library-site` gives `warning content/_index.md: links to /posts, but no page has that address` for `page-teasers section="/posts"` (`testdata/library-site/content/_index.md:46`). The save path accepts that value and the theme renders it. - The PR follows the brief's "exact permalink" rule as written, and `/api/link/resolve` already answered `found: false` for this value. Still, the engine now gives two different answers to "does this value name an existing page": `element.pageKind.site` allows the slash, `linkresolve` does not. The brief also says the check produces no false positives. - The fix suggested by the warning, writing `/posts/`, is harmless. The maintainer should decide whether the owner adopts the trailing-slash allowance for `page`-typed values, or the warning stands and the legacy shape is migrated. **Minor, related (already disclosed in the PR body):** a link to a file under `static/` (e.g. `/images/x.png`) gets the misleading `the bundle of / has no file "images/x.png"` (`internal/linkresolve/linkresolve.go:184`). No scaffold ships `static/`, but a migrated site could, so this is worth a follow-up issue.

dominik.polakovics commented 2026-10-15 15:52:46 +02:00

Author

Owner

Copy link

Pushed a third commit addressing the review's concern directly rather than as a follow-up: the page field type's save check no longer carries a trailing-slash tolerance of its own. `linkresolve.Resolver.Page` (exact permalink of a real page) is exported, `pageops`' `element.Site` calls it, and `TypePage` drops the retry — so save, `harness check`, resolve, publish and the rename rewrite all give one answer for `/posts`.

Consequences, in the PR body's terms:

• The "Worth knowing" note about `testdata/library-site` is obsolete: the fixture now stores `/posts/` and `harness check` on it reports no link warning.
• A legacy slashless `page` value in existing content is a dangling reference: `harness check` lists it, and the element's next save is refused until the page is re-picked. There is no automatic content migration in this PR; the check is the migration guidance. AGENTS.md ("page" under the sidecar schema), the page-teasers and news-list sidecar comments, the page-teasers template comment and the add-element reference say so instead of promising the tolerance.
• `go vet ./...` and `go test ./...` pass with `web/dist` built and Hugo 0.167.0 on PATH (the theme test over the library fixture included); `gofmt` is clean.

Pushed a third commit addressing the review's concern directly rather than as a follow-up: the page field type's save check no longer carries a trailing-slash tolerance of its own. \`linkresolve.Resolver.Page\` (exact permalink of a real page) is exported, \`pageops\`' \`element.Site\` calls it, and \`TypePage\` drops the retry — so save, \`harness check\`, resolve, publish and the rename rewrite all give one answer for \`/posts\`. Consequences, in the PR body's terms: - The "Worth knowing" note about \`testdata/library-site\` is obsolete: the fixture now stores \`/posts/\` and \`harness check\` on it reports no link warning. - A legacy slashless \`page\` value in existing content is a dangling reference: \`harness check\` lists it, and the element's next save is refused until the page is re-picked. There is no automatic content migration in this PR; the check is the migration guidance. AGENTS.md ("page" under the sidecar schema), the page-teasers and news-list sidecar comments, the page-teasers template comment and the add-element reference say so instead of promising the tolerance. - \`go vet ./...\` and \`go test ./...\` pass with \`web/dist\` built and Hugo 0.167.0 on PATH (the theme test over the library fixture included); \`gofmt\` is clean.

dominik.polakovics added 1 commit 2026-10-15 15:52:46 +02:00

fix(element): the page field type resolves through linkresolve, no slash tolerance ... e101d17467

"Does this stored value name a page" was answered twice: the page field
type's save check tolerated a value missing only its trailing slash
("/posts" for "/posts/", the hand-typed shape of params upgraded from
text), while the link check, GET /api/link/resolve, publish and the
rename rewrite all held the exact permalink. So `harness check` warned
on a value the same engine saved, and a rename of the section silently
left the value behind.

One owner now: linkresolve.Resolver.Page (the exact permalink of a real
page) is exported, pageops' element.Site calls it, and TypePage drops
its retry. A legacy slashless value is a dangling reference everywhere:
`harness check` reports it and the element's next save is refused until
the page is picked again. AGENTS.md, the page-teasers and news-list
sidecars, the page-teasers template comment and the add-element
reference stop promising the tolerance; the library fixture stores the
canonical form, so the theme test pins the render contract alone.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

dominik.polakovics added 1 commit 2026-10-15 17:13:59 +02:00

Merge branch 'main' into afk/195 ... 33c9f4f8f7

Brings in #306 (machine-owned, versioned sidecars and harness migrate).

Conflicts resolved:
- scaffold/editor/elements/page-teasers.yaml and
  scaffold/features/news/editor/elements/news-list.yaml: main's canonical,
  machine-owned form. This branch only edited their prose comments (to drop
  the "/posts still resolves" promise); main replaced those comments with the
  machine-owned header, so the promise is gone either way.
- scaffold/site/AGENTS.md "Verifying a change" and the verify-site skill:
  union of both lists (outdated/non-canonical sidecars from main, dangling
  links from this branch).
- docs/harness.md: the "Links." paragraph auto-merged below main's new
  "## Migrate" section; moved back to the end of "## Check", which its
  "see the next paragraph" pointer refers to.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

dominik.polakovics commented 2026-10-15 17:14:08 +02:00

Author

Owner

Copy link

This was generated by AI while landing a PR.

Merged main (081a4c3, #306 — machine-owned, versioned sidecars and harness migrate) into afk/195 as 33c9f4f; four conflicts, all resolved without a semantic choice:

• scaffold/editor/elements/page-teasers.yaml, scaffold/features/news/editor/elements/news-list.yaml: took main's canonical form. This branch only edited the prose comments (dropping the "/posts still resolves" promise); main replaced those comments with the machine-owned header, so the promise is gone either way and the shipped-sidecar canonical test holds.
• scaffold/site/AGENTS.md "Verifying a change" and the verify-site skill: union of both lists (outdated / non-canonical sidecars from main, dangling links from this branch).
• docs/harness.md: git auto-merged the Links. paragraph below main's new ## Migrate section; moved it back to the end of ## Check, where the warnings list's "see the next paragraph" points.

Re-verified on the merged tree: gofmt clean, go vet ./..., go test ./... (all packages, drift tests included, Hugo 0.167.0 on PATH, web/dist built) and go build ./cmd/harness pass. Smoke: fresh harness init → no problems; the issue's reproduction → exactly the three link warnings, exit 0.

> *This was generated by AI while landing a PR.* Merged `main` (081a4c3, #306 — machine-owned, versioned sidecars and `harness migrate`) into `afk/195` as 33c9f4f; four conflicts, all resolved without a semantic choice: - `scaffold/editor/elements/page-teasers.yaml`, `scaffold/features/news/editor/elements/news-list.yaml`: took `main`'s canonical form. This branch only edited the prose comments (dropping the "`/posts` still resolves" promise); `main` replaced those comments with the machine-owned header, so the promise is gone either way and the shipped-sidecar canonical test holds. - `scaffold/site/AGENTS.md` "Verifying a change" and the `verify-site` skill: union of both lists (outdated / non-canonical sidecars from `main`, dangling links from this branch). - `docs/harness.md`: git auto-merged the **Links.** paragraph below `main`'s new `## Migrate` section; moved it back to the end of `## Check`, where the warnings list's "see the next paragraph" points. Re-verified on the merged tree: `gofmt` clean, `go vet ./...`, `go test ./...` (all packages, drift tests included, Hugo 0.167.0 on PATH, `web/dist` built) and `go build ./cmd/harness` pass. Smoke: fresh `harness init` → no problems; the issue's reproduction → exactly the three link warnings, exit 0.

dominik.polakovics commented 2026-10-15 17:17:43 +02:00

Author

Owner

Copy link

[autoland] verdict: pass

This was generated by AI while landing a PR.

Verdict: CONCERNS (non-blocking) — validated, awaiting the maintainer's merge confirmation.

Signal relied on: ci / build-and-test green on 33c9f4f (run 312: frontend build + tests, go vet ./..., go test ./... incl. the drift tests, harness build), plus the same Go suite run locally on the resolved tree (the merge-from-main commit, see the comment above). Title is Conventional Commits; body carries Closes #195.

Checked against #195's brief: one owner, internal/linkresolve, called by harness check, GET /api/link/resolve and publish's draft-link check; no copy left in internal/server or internal/checkcmd. Publish's content-path match is gone. Resolve answers found: true for an existing bundle file. Every acceptance criterion has a test, and the issue's reproduction gives exactly three warnings, exit 0. Docs and skills are updated: docs/harness.md Check, docs/api.md, AGENTS.md, verify-site, migrate-site, both linkrefs comments.

Beyond the brief, maintainer-owned: commit e101d17 removes the page field type's trailing-slash tolerance (internal/element/ftype_page.go), so a legacy /posts value is now refused on save until the page is picked again, and harness check lists it. This resolves the earlier review's "two answers" concern and is documented in AGENTS.md, the sidecar and template comments and the add-element reference. Noted as a deliberate behaviour change for existing sites. It is not a concern.

Concern: false positive for static/ files. A link to a real file under static/ warns. Reproduced: static/files/brochure.pdf linked as /files/brochure.pdf gives links to /files/brochure.pdf, but the bundle of / has no file "files/brochure.pdf". This comes from internal/linkresolve/linkresolve.go Resolve, where the home page's branch bundle is the prefix of every target. The brief asks for no false positives. It is only a warning, no scaffold ships static/, and it is disclosed in the PR body, but a migrated site may have such files. Worth a follow-up issue.

[autoland] verdict: pass > *This was generated by AI while landing a PR.* **Verdict: CONCERNS (non-blocking)** — validated, awaiting the maintainer's merge confirmation. **Signal relied on:** `ci / build-and-test` green on 33c9f4f (run 312: frontend build + tests, `go vet ./...`, `go test ./...` incl. the drift tests, `harness` build), plus the same Go suite run locally on the resolved tree (the merge-from-`main` commit, see the comment above). Title is Conventional Commits; body carries `Closes #195`. **Checked against #195's brief:** one owner, `internal/linkresolve`, called by `harness check`, `GET /api/link/resolve` and publish's draft-link check; no copy left in `internal/server` or `internal/checkcmd`. Publish's content-path match is gone. Resolve answers `found: true` for an existing bundle file. Every acceptance criterion has a test, and the issue's reproduction gives exactly three warnings, exit 0. Docs and skills are updated: `docs/harness.md` Check, `docs/api.md`, AGENTS.md, `verify-site`, `migrate-site`, both `linkrefs` comments. **Beyond the brief, maintainer-owned:** commit e101d17 removes the `page` field type's trailing-slash tolerance (`internal/element/ftype_page.go`), so a legacy `/posts` value is now refused on save until the page is picked again, and `harness check` lists it. This resolves the earlier review's "two answers" concern and is documented in AGENTS.md, the sidecar and template comments and the add-element reference. Noted as a deliberate behaviour change for existing sites. It is not a concern. **Concern: false positive for `static/` files.** A link to a real file under `static/` warns. Reproduced: `static/files/brochure.pdf` linked as `/files/brochure.pdf` gives `links to /files/brochure.pdf, but the bundle of / has no file "files/brochure.pdf"`. This comes from `internal/linkresolve/linkresolve.go` `Resolve`, where the home page's branch bundle is the prefix of every target. The brief asks for no false positives. It is only a warning, no scaffold ships `static/`, and it is disclosed in the PR body, but a migrated site may have such files. Worth a follow-up issue.

dominik.polakovics merged commit 40897188ea into main 2026-10-15 17:19:44 +02:00

dominik.polakovics referenced this pull request from a commit 2026-10-15 17:19:44 +02:00

Merge pull request 'feat(check): warn on internal links that do not resolve' (#301) from afk/195 into main

dominik.polakovics referenced this pull request 2026-10-16 18:02:45 +02:00

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

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bug

Something is broken

  

enhancement

New feature or improvement

  

needs-triage

Maintainer needs to evaluate this issue

  

ready-for-agent

Fully specified, ready for an AFK agent

  

ready-for-human

Requires human implementation

No labels

bug

enhancement

needs-triage

ready-for-agent

ready-for-human

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Cloonar/deckle!301

No description provided.