512 lines
30 KiB
Markdown
512 lines
30 KiB
Markdown
---
|
|
phase: 11.1-summercms-documentation-for-humans-and-ai-agents
|
|
reviewed: 2026-10-01T07:20:12Z
|
|
depth: standard
|
|
files_reviewed: 95
|
|
files_reviewed_list:
|
|
- cmd/summer/docs.go
|
|
- cmd/summer/docs_test.go
|
|
- cmd/summer/main.go
|
|
- cmd/summer/main_test.go
|
|
- cmd/summer/phase11_1_acceptance_test.go
|
|
- docs/examples/blog/blog_test.go
|
|
- docs/examples/blog/classes/doc.go
|
|
- docs/examples/blog/console/doc.go
|
|
- docs/examples/blog/console/publish.go
|
|
- docs/examples/blog/console/publish_test.go
|
|
- docs/examples/blog/controllers/doc.go
|
|
- docs/examples/blog/controllers/posts.go
|
|
- docs/examples/blog/controllers/posts_test.go
|
|
- docs/examples/blog/jobs/doc.go
|
|
- docs/examples/blog/middleware/doc.go
|
|
- docs/examples/blog/models/doc.go
|
|
- docs/examples/blog/models/post.go
|
|
- docs/examples/blog/models/post_test.go
|
|
- docs/examples/blog/plugin.go
|
|
- docs/examples/blog/postgres_test.go
|
|
- docs/examples/blog/registry.gen.go
|
|
- docs/examples/blog/routes.go
|
|
- docs/examples/blog/scaffold_layout_test.go
|
|
- docs/examples/blog/updates/20260101000000_create_acme_blog_posts.go
|
|
- docs/examples/blog/updates/20260101000100_add_published_at.go
|
|
- docs/examples/blog/updates/doc.go
|
|
- docs/examples/blog/updates/updates_test.go
|
|
- internal/docsite/check_commands.go
|
|
- internal/docsite/check_forbidden.go
|
|
- internal/docsite/check_identifiers.go
|
|
- internal/docsite/check_links.go
|
|
- internal/docsite/check_policy.go
|
|
- internal/docsite/checks_test.go
|
|
- internal/docsite/docsite.go
|
|
- internal/docsite/docsite_test.go
|
|
- internal/docsite/emit.go
|
|
- internal/docsite/emit_test.go
|
|
- internal/docsite/highlight.go
|
|
- internal/docsite/highlight_test.go
|
|
- internal/docsite/load.go
|
|
- internal/docsite/load_test.go
|
|
- internal/docsite/render.go
|
|
- internal/docsite/render_test.go
|
|
- internal/docsite/serve.go
|
|
- internal/docsite/serve_test.go
|
|
- internal/docsite/snippet.go
|
|
- internal/docsite/snippet_test.go
|
|
- internal/docsite/theme/assets/search.js
|
|
- internal/docsite/theme/assets/site.css
|
|
- internal/docsite/theme/assets/site.js
|
|
- internal/docsite/theme/assets/theme-init.js
|
|
- internal/docsite/theme/templates/404.html
|
|
- internal/docsite/theme/templates/footer.html
|
|
- internal/docsite/theme/templates/header.html
|
|
- internal/docsite/theme/templates/icons.html
|
|
- internal/docsite/theme/templates/page.html
|
|
- internal/docsite/theme/templates/pager.html
|
|
- internal/docsite/theme/templates/search.html
|
|
- internal/docsite/theme/templates/sidebar.html
|
|
- internal/docsite/theme/templates/toc.html
|
|
- internal/docsite/theme_test.go
|
|
- internal/docsite/violations_test.go
|
|
- modules/backpack/example_test.go
|
|
- modules/beachcomber/example_test.go
|
|
- modules/beachcomber/export_docs_test.go
|
|
- modules/beachcomber/typesense/example_test.go
|
|
- modules/bonfire/example_test.go
|
|
- modules/bouncer/example_test.go
|
|
- modules/cabana/example_controller_test.go
|
|
- modules/cabana/example_test.go
|
|
- modules/compass/example_test.go
|
|
- modules/conga/example_test.go
|
|
- modules/conga/export_docs_test.go
|
|
- modules/festival/example_test.go
|
|
- modules/fetchguard/example_test.go
|
|
- modules/flare/example_test.go
|
|
- modules/lagoon/attach/example_test.go
|
|
- modules/lagoon/example_test.go
|
|
- modules/lagoon/export_docs_test.go
|
|
- modules/lighthouse/centrifugo/example_test.go
|
|
- modules/lighthouse/example_test.go
|
|
- modules/lighthouse/export_docs_test.go
|
|
- modules/pact/example_test.go
|
|
- modules/party/example_plugin_test.go
|
|
- modules/party/example_test.go
|
|
- modules/phrasebook/example_test.go
|
|
- modules/postcard/example_test.go
|
|
- modules/surf/example_test.go
|
|
- modules/tide/example_test.go
|
|
- modules/towel/example_test.go
|
|
- modules/wire/example_test.go
|
|
- modules/wristband/example_test.go
|
|
- scripts/check-phase11.1.sh
|
|
- internal/docsite/fences.go
|
|
- internal/docsite/fences_test.go
|
|
findings:
|
|
critical: 1
|
|
warning: 7
|
|
info: 8
|
|
total: 16
|
|
status: issues_found
|
|
---
|
|
|
|
# Phase 11.1: Code Review Report
|
|
|
|
**Reviewed:** 2026-09-30T22:32:58Z
|
|
**Depth:** standard
|
|
**Files Reviewed:** 93
|
|
**Status:** issues_found
|
|
|
|
## Summary
|
|
|
|
The review covered the docs generator (`internal/docsite`), the `summer docs:*` commands, the theme (templates and JS), the phase gate script, the acme/blog walkthrough package and the new module example tests.
|
|
|
|
The security-sensitive surfaces the orchestrator named hold up:
|
|
|
|
- **docs:serve.** It refuses non-loopback addresses unless `--allow-remote` is passed. The handler rejects every dot segment. Paths are cleaned and checked lexically against the build dir. The build dir is written only by `writeOutputs`, which creates no symlinks.
|
|
- **--out guard.** It resolves symlinks, and `prepareOut` refuses to empty any unmarked non-empty directory.
|
|
- **XSS.** `search.js` builds results with `createElement` and `textContent` only. The templates go through `html/template`, and goldmark runs in safe mode.
|
|
- **blog:publish.** The command binds the slug as a parameter. The admin controller's `acme.blog.access_posts` permission is registered in `Permissions()` and is enforced by cabana (`http.go:754`).
|
|
|
|
The main problems are checkers that fail open. Each finding below was reproduced with the built `summer` binary against a scratch copy of `internal/docsite/testdata/clean`:
|
|
|
|
- A `src=` fence inside a blockquote or callout is published with a source caption, but its body is never compared with the source.
|
|
- An Example that has no `// Output:` comment, and every helper it calls, counts as "run by go test".
|
|
- Files excluded by build constraints count as "compiled".
|
|
- The `go doc` fallback matches case-insensitively, so identifiers with the wrong case pass.
|
|
- `golang` fences and several command-line forms are not checked at all.
|
|
|
|
There is also one latent theme bug. A heading whose slug matches a theme element ID breaks search on that page.
|
|
|
|
## Critical Issues
|
|
|
|
### CR-01: `src=` and Go fences inside blockquotes or callouts bypass snippet verification but are still published as "verified source"
|
|
|
|
**File:** `internal/docsite/render.go:439-489` (scanFences/openFence), used by `internal/docsite/snippet.go:451-485` (checkFences), `internal/docsite/check_policy.go:399-415` and `internal/docsite/check_commands.go:89-102`. The annotation happens in `internal/docsite/render.go:54-72`.
|
|
|
|
**Issue:** Every text-level checker finds fences with `scanFences`. It strips only leading spaces, so it never sees a fence whose lines begin with `> `. That excludes any code block inside a `> [!NOTE]`, `> [!TIP]` or `> [!WARNING]` callout or a plain blockquote. goldmark does parse such a fence. `fenceAnnotator` walks the AST and adds `data-src`/`data-href`, so the rendered page shows a `<figcaption>` that names and links the source file, even though the body was never compared with it.
|
|
|
|
Reproduced on the clean fixture: the following text was appended to `docs/extras/faq.md`.
|
|
|
|
```
|
|
> [!TIP]
|
|
> ```go src=modules/demo/example_test.go#ExampleHello
|
|
> BOGUS
|
|
> ```
|
|
```
|
|
|
|
`summer docs:build --check` printed "no problems found". `docs:build` then wrote `extras/faq.html` containing `BOGUS` under a caption for `modules/demo/example_test.go#ExampleHello`. A `src=` path that does not exist also passes, and so does a go fence with no `src=` inside a callout.
|
|
|
|
There is a second path with the same result. `checkSnippets` skips module README pages (`snippet.go:509`), but `fenceAnnotator` still annotates their `src=` fences.
|
|
|
|
The acceptance test's independent scanner (`cmd/summer/phase11_1_acceptance_test.go:418-442`) has the same blind spot, so SC4 cannot catch this either. Together these defeat DOCS-04, the promise that every Go example on the site is compiled and run by go test, without any warning. `docs:sync` will not repair such fences either.
|
|
|
|
**Fix:** Collect fences from the goldmark AST that `parseDocs` already produces, instead of scanning lines. Use `*ast.FencedCodeBlock` nodes with `Info` and `Lines()`, so blockquotes and list items are handled the way the renderer handles them. At minimum, refuse what the line scanner cannot verify:
|
|
|
|
```go
|
|
// in checkPolicy, over d.doc:
|
|
_ = gast.Walk(d.doc, func(n gast.Node, entering bool) (gast.WalkStatus, error) {
|
|
fc, ok := n.(*gast.FencedCodeBlock)
|
|
if !entering || !ok || fc.Info == nil {
|
|
return gast.WalkContinue, nil
|
|
}
|
|
info := string(fc.Info.Segment.Value(d.body))
|
|
_, hasSrc := ParseSrc(info)
|
|
if hasSrc && (d.page.Module != "" || insideBlockquote(fc)) {
|
|
problems = append(problems, Problem{File: d.file, Line: lineOf(d.body, fc.Info.Segment.Start, d.line),
|
|
Rule: "snippet", Message: "src= fences must be top-level blocks of a docs page"})
|
|
}
|
|
return gast.WalkContinue, nil
|
|
})
|
|
```
|
|
|
|
Also make `fenceAnnotator` skip README pages, or run `checkFences` for them too. Update the acceptance scanner to strip `>` prefixes.
|
|
|
|
## Warnings
|
|
|
|
### WR-01: An Example without `// Output:` is a reachability root, so code go test never runs is accepted as "run"
|
|
|
|
**File:** `internal/docsite/snippet.go:344-346` (isRoot), `:408-441` (checkTestIdent/checkTestRegion)
|
|
|
|
**Issue:** `isRoot` treats every `Example*` function as a root of the test graph. `go test` compiles an Example with no `// Output:` comment but never runs it. When `Extract` targets an Example directly, it checks for the output comment (`exampleHasOutput`). Helpers and regions reached only through such an Example pass `checkTestIdent`/`checkTestRegion` anyway.
|
|
|
|
Reproduced: `func Example_neverRun() { neverRunHelper() }` without an output comment was added. After that, `src=…#neverRunHelper` and a `docs:start never` region inside the helper both passed `docs:build --check`. `go test -v` shows the Example never runs.
|
|
|
|
Code after an unconditional `t.Skip` also passes, for example a region inside a `TestSkipped` that calls `t.Skip("always")` first. That case is harder to close, but it should at least be documented.
|
|
|
|
**Fix:** When building the graph, count an Example as a root only when `doc.Examples(f)` reports output for it. Parse the test files with `parser.ParseComments` so that information is available:
|
|
|
|
```go
|
|
roots := map[string]bool{}
|
|
for _, ex := range doc.Examples(f) {
|
|
if ex.Output != "" || ex.EmptyOutput {
|
|
roots["Example"+ex.Name] = true
|
|
}
|
|
}
|
|
isRoot := func(d *ast.FuncDecl) bool {
|
|
return d.Recv == nil && (strings.HasPrefix(d.Name.Name, "Test") || roots[d.Name.Name])
|
|
}
|
|
```
|
|
|
|
### WR-02: Go sources excluded by build constraints pass the "compiled by go test" check
|
|
|
|
**File:** `internal/docsite/snippet.go:102-114`
|
|
|
|
**Issue:** For a non-test `.go` file, `Extract` checks only two things: the file sits in the root module, and a `*_test.go` exists beside it. It ignores `//go:build` constraints and `_GOOS`/`_GOARCH` file-name suffixes. It also ignores directories that the go tool skips (`testdata`, `_x`, `.x`) when they hold their own `_test.go` files.
|
|
|
|
Reproduced: `modules/demo/broken.go` was created with `//go:build ignore` and `func Broken() { undefinedCall() }`. `src=modules/demo/broken.go#Broken` synced and passed `docs:build --check`, and `go vet ./...` stayed green. Code that does not compile was published as verified.
|
|
|
|
**Fix:** Use `go/build` to confirm the file is part of the package under the default context:
|
|
|
|
```go
|
|
pkg, err := build.ImportDir(filepath.Dir(real), 0)
|
|
if err != nil || !slices.Contains(append(pkg.GoFiles, pkg.TestGoFiles...), filepath.Base(real)) &&
|
|
!slices.Contains(pkg.XTestGoFiles, filepath.Base(real)) {
|
|
return "", snippetError("file is excluded from the default build (build constraints or ignored directory)")
|
|
}
|
|
```
|
|
|
|
### WR-03: The `go doc` fallback accepts identifiers with the wrong case
|
|
|
|
**File:** `internal/docsite/check_identifiers.go:301`
|
|
|
|
**Issue:** When a lookup misses the index, `goDoc` runs `go doc ./<dir> <query>`. In `go doc`, a lowercase letter in the query matches either case. Reproduced: with `func OpenFromApp()` in the fixture, the span `` `demo.Openfromapp` `` passed `docs:build --check`. On the real tree, `go doc ./modules/lagoon Openfromapp` exits 0. A page can therefore name an API that does not compile, and the identifier rule reports nothing.
|
|
|
|
**Fix:** Pass `-c` so symbol matching is case-sensitive:
|
|
|
|
```go
|
|
cmd := exec.CommandContext(ctx, "go", "doc", "-c", "./"+dir, query)
|
|
```
|
|
|
|
### WR-04: `golang` fences (and other Go aliases) bypass the "go fence needs src=" policy
|
|
|
|
**File:** `internal/docsite/check_policy.go:409-414`
|
|
|
|
**Issue:** The policy checks only `fields[0] == "go"`. chroma resolves `golang` to the Go lexer, so a ```` ```golang ```` fence renders as highlighted Go with no `src=` requirement at all. Reproduced: `` ```golang\nx := 1\n``` `` passed `docs:build --check`. The acceptance test (`docsGoFences`, `lang == "go"`) misses it too.
|
|
|
|
**Fix:** Normalise the language through chroma before the comparison:
|
|
|
|
```go
|
|
if l := lexers.Get(fields[0]); l != nil && l.Config().Name == "Go" && !hasSrc { ... }
|
|
```
|
|
|
|
Or reject any fence whose lexer is Go but whose language word is not exactly `go`.
|
|
|
|
### WR-05: The command checker skips common shell forms, so unknown commands are published unchecked
|
|
|
|
**File:** `internal/docsite/check_commands.go:35`, `:66-80`
|
|
|
|
**Issue:** `commandToken` matches only `summer <name>` or `./bin/<app> <name>`, and only at the start of a command. Reproduced: the following forms inside an `sh` fence all passed `--check`:
|
|
|
|
- `FOO=1 summer no:such` (an environment-variable prefix)
|
|
- `summer --root . no:such` (a flag before the command name, which is skipped because `m[2]` starts with `-`)
|
|
- `go run ./cmd/summer no:such`
|
|
|
|
`bin/acme migrate`, without the leading `./`, and `text` fences are never checked either. Any of these forms is a natural way to write a command, and a typo in one ships without a warning.
|
|
|
|
**Fix:** Strip leading `VAR=value` assignments and a `go run ./cmd/summer` prefix. Skip flags before the command word, and treat a flag's value as consumed when the flag is known to take one. Accept `bin/<app>` as well as `./bin/<app>`. Add planted cases under `testdata/violations/` for each form.
|
|
|
|
### WR-06: Heading IDs can collide with theme element IDs, which breaks search on that page and produces invalid HTML
|
|
|
|
**File:** `internal/docsite/render.go:207-232` (slugIDs); theme IDs in `templates/page.html:36`, `templates/sidebar.html:2`, `templates/search.html:2-6`, `templates/page.html:27`
|
|
|
|
**Issue:** `slugIDs` starts every page with an empty `used` set. A heading such as `## Search`, `## Content`, `## Sidebar` or `## Search results` therefore gets the same `id` as a theme element. Reproduced: the built `extras/faq.html` contained `id="search"` twice, first on the `<h2>`, which comes before the `<dialog>` in document order.
|
|
|
|
`search.js:10` calls `document.getElementById("search")` and gets the `<h2>`. The guard `typeof dialog.showModal !== "function"` then disables search on that page without any message. The skip link `#content` jumps to the heading instead of `<main>`. `search-results` would make `fetch(null)` report "Search needs a web server". None of these headings exist in the current tree, but nothing prevents them.
|
|
|
|
**Fix:** Reserve the theme IDs in every page's slug table. Because the link checker uses the same parser, it will stay consistent:
|
|
|
|
```go
|
|
var themeIDs = []string{"content", "sidebar", "search", "search-input", "search-status",
|
|
"search-results", "search-live", "live-region"}
|
|
|
|
func newSlugIDs() *slugIDs {
|
|
s := &slugIDs{used: map[string]bool{}}
|
|
for _, id := range themeIDs {
|
|
s.used[id] = true
|
|
}
|
|
return s
|
|
}
|
|
```
|
|
|
|
Alternatively, prefix the theme IDs (`sd-search`, …) in the templates and JS.
|
|
|
|
### WR-07: The walkthrough's hand-written files carry "Code generated … DO NOT EDIT" headers
|
|
|
|
**File:** `docs/examples/blog/console/publish.go:1`, `docs/examples/blog/controllers/posts.go:1`, `docs/examples/blog/models/post.go:1`, `docs/examples/blog/updates/*.go:1`
|
|
|
|
**Issue:** These files contain hand-written logic: the `withDB` parameter and the COALESCE update in `publish.go`, `RequiredPermissions`/`NewRecord` in `posts.go`, and `Fillable`/`Rules`/`NewPost` in `post.go`. They still keep the generated-code header. By default, golangci-lint (named in the stack) and staticcheck skip files marked as generated, so the only hand-written business logic in the walkthrough is never linted. The walkthrough page also teaches readers to edit files marked DO NOT EDIT. The scaffolder gap is tracked in `.planning/todos/pending/scaffold-generated-header-and-command-deps.md`, but the example should not model the problem.
|
|
|
|
**Fix:** Remove the header from files that have been finished by hand, and say so in `porting-a-plugin.md`. Keep it only on `registry.gen.go`, which the scaffolder really regenerates. If the scaffolder is going to keep emitting the header, add a sentence to the page telling readers to delete it once they edit the file.
|
|
|
|
## Info
|
|
|
|
### IN-01: A failed write leaves an unmarked output directory that the next build refuses to clean
|
|
|
|
**File:** `internal/docsite/docsite.go:95-103`
|
|
|
|
**Issue:** `prepareOut` deletes everything in the output directory, including the `.summer-docs` marker. The marker is written again only after every output succeeds. If a write fails partway (for example ENOSPC or EACCES), the directory is left non-empty and unmarked. Every later `docs:build` then stops with "refusing to clean … it has no .summer-docs marker".
|
|
|
|
**Fix:** Write the marker right after `prepareOut` succeeds and before `writeOutputs`.
|
|
|
|
### IN-02: `scanFences` treats 4-space-indented backticks as fenced code, which gives false positives and differs from goldmark
|
|
|
|
**File:** `internal/docsite/render.go:462-479`
|
|
|
|
**Issue:** `openFence` strips any number of leading spaces. Top-level text such as ` ```go` is an indented code block in CommonMark, but it was reported as "go code block has no src= reference" (reproduced).
|
|
|
|
**Fix:** Limit the fence indent to 3 spaces relative to the enclosing block, or take fences from the AST (see CR-01).
|
|
|
|
### IN-03: `data-href` from `source_url` is HTML-escaped but its URL scheme is not checked
|
|
|
|
**File:** `internal/docsite/highlight.go:45-46`, `internal/docsite/render.go:67-69`
|
|
|
|
**Issue:** `edit_url` goes through `html/template`, which neutralises `javascript:` URLs. `source_url` is written into a raw `href` with only `html.EscapeString`. `site.yaml` is trusted, but the two URL paths behave inconsistently.
|
|
|
|
**Fix:** Validate `source_url` in `ParseSite`: require it to start with `http://` or `https://`, and do the same for `edit_url`.
|
|
|
|
### IN-04: Dead error branch and a nil request in serve.go
|
|
|
|
**File:** `internal/docsite/serve.go:79-85`, `:283`
|
|
|
|
**Issue:** `s.watch` only ever returns nil, so the `watchErr` branch that handles non-nil errors can never run. `http.NotFound(w, nil)` passes a nil `*http.Request`. It works only because the current stdlib ignores the argument.
|
|
|
|
**Fix:** Remove the unreachable branch, or make `watch` return real errors. Pass `r` down to `notFound`.
|
|
|
|
### IN-05: Search highlighting uses offsets from `toLowerCase()` on the original string
|
|
|
|
**File:** `internal/docsite/theme/assets/search.js:196-236`
|
|
|
|
**Issue:** `appendMarked` and `excerpt` look up indices in `text.toLowerCase()` and then slice `text`. For characters whose lowercase form has a different length (for example U+0130 "İ"), the `<mark>` ranges shift. This is not an XSS risk, because `textContent` is still used throughout.
|
|
|
|
**Fix:** Compute the indices on a lowercased copy only when `lower.length === text.length`, and otherwise skip highlighting.
|
|
|
|
### IN-06: The gate's forbidden sweep treats grep errors as "no hits"
|
|
|
|
**File:** `scripts/check-phase11.1.sh:131-133`
|
|
|
|
**Issue:** `grep -rliE … 2>/dev/null || true` discards grep's exit status 2 (for example an unreadable file or a missing path). A failed sweep therefore reads as clean. The Go checker covers the same rule, so the risk is limited to this redundant stage.
|
|
|
|
**Fix:**
|
|
|
|
```bash
|
|
forbidden_hits() {
|
|
local rc=0
|
|
grep -rliE "$FORBIDDEN_RE" "$@" || rc=$?
|
|
[ "$rc" -le 1 ] || refuse "forbidden sweep: grep failed ($rc)"
|
|
}
|
|
```
|
|
|
|
### IN-07: The gate depends on GNU-only tools
|
|
|
|
**File:** `scripts/check-phase11.1.sh:285`, `:289`, `:376`
|
|
|
|
**Issue:** `sed -i` without a suffix argument and `find -printf` fail on BSD/macOS. Under `set -e` that surfaces as an unexplained exit.
|
|
|
|
**Fix:** Use `sed -i.bak … && rm ….bak` (or perl), and `find … -exec dirname {} \;`, or document that GNU userland is required.
|
|
|
|
### IN-08: The identifier checker cannot see several span forms
|
|
|
|
**File:** `internal/docsite/check_identifiers.go:223`, `:233-236`
|
|
|
|
**Issue:** A span whose package prefix is not a module directory name returns "" with no check, so a typo like `` `lagon.Fill` `` passes. `&lagoon.Foo{}`, `lagoon.Foo{…}` and `x := lagoon.Foo()` do not match `identSpan` either. `docs:serve` also does not watch the root `README.md` or `examples/`, although both feed the checks.
|
|
|
|
**Fix:** Extend `identSpan` to allow a leading `&` and a trailing `{…}`. Consider flagging lowercase `pkg.Ident` spans whose `pkg` is close to a module name, for example by edit distance. Add the root README and `examples/` to `addWatches`.
|
|
|
|
---
|
|
|
|
_Reviewed: 2026-09-30T22:32:58Z_
|
|
_Reviewer: Claude (gsd-code-reviewer)_
|
|
_Depth: standard_
|
|
|
|
## Gap closure 11.1-07
|
|
|
|
Reviewed the gap-closure diff `d9f187ae..HEAD` (standard depth) on 2026-10-01T07:20:12Z. Scope was the fence AST, snippet execution check, command-word parser, `go doc -c`, the acceptance scanner, and the phase-gate plants. The sections above are the 2026-09-30 review and are unchanged.
|
|
|
|
This pass: critical 0, warning 4, info 1, total 5.
|
|
|
|
### Prior findings touched by this diff
|
|
|
|
- **CR-01 fixed in the product.** `collectFences` walks goldmark's `*ast.FencedCodeBlock` nodes, so callouts, blockquotes and list items are visible. A nested `src=` fence is a snippet problem and is not rewritten by `docs:sync`. `fenceAnnotator` captions only a top-level `src=` fence of a docs page, so a module README or a nested fence no longer renders a verified-source caption. The acceptance scanner strips `>` markers. One list form is still invisible to that scanner (WR-11); `docs:build --check` already refuses it.
|
|
- **WR-01 fixed for Examples.** `loadTestGraph` roots an Example only when `doc.Examples` reports `Output` or `EmptyOutput`. A helper or region reached only from an Example with no output is refused. The original note about an unconditional `t.Skip` is unchanged; this diff does not address it.
|
|
- **WR-02 fixed for the cases it reproduced.** `//go:build ignore`, a mismatched `_GOOS` file, and a lexical `testdata` or `_`-prefixed segment are refused. A directory symlink into `testdata`, and a package under `vendor/`, still pass (WR-09).
|
|
- **WR-03 fixed.** `goDoc` runs `go doc -c`.
|
|
- **WR-04 fixed.** `goLang` uses the chroma lexer, so `golang`, `GO` and `main.go` need `src=`.
|
|
- **WR-05 fixed for the forms it named** (leading `VAR=value`, a flag before the command word, `go run ./cmd/summer`, `bin/{app}` without `./`). Shell language case and aliases, and a continuation backslash, are still open (WR-08, WR-10).
|
|
- **WR-06, WR-07, IN-01 through IN-08** are not changed by this diff.
|
|
|
|
### Warnings
|
|
|
|
### WR-08: Shell fences in any language other than the exact words `sh`, `shell`, `bash` and `console` are not command-checked
|
|
|
|
**File:** `internal/docsite/check_commands.go:31` (`shellLangs`), used at `internal/docsite/check_commands.go:89`
|
|
|
|
**Issue:** Go fences were moved onto chroma's lexer registry so `golang` and `GO` cannot skip the policy. Shell fences were not. `slices.Contains(shellLangs, f.lang)` is a case-sensitive compare against four lowercase words. chroma classifies `Bash`, `SH`, `Shell` and `zsh` as the Bash lexer and `sh-session` as Bash Session, and `highlight` will colour them as shell, but `checkCommands` skips the fence. Reproduced: a docs page whose only shell sample was a `Bash` fence containing `summer no:such` produced no command problem from `Check`. The same page's `sh` fence was checked. An unknown command written the way the Go aliases used to be written still ships.
|
|
|
|
**Fix:** Treat a fence as shell when its language is in `shellLangs` or when `lexerFor` reports the Bash or Bash Session lexer, and keep `console` in that set:
|
|
|
|
```go
|
|
func shellFence(lang string) bool {
|
|
if slices.Contains(shellLangs, lang) {
|
|
return true
|
|
}
|
|
l := lexerFor(lang)
|
|
if l == nil {
|
|
return false
|
|
}
|
|
switch l.Config().Name {
|
|
case "Bash", "Bash Session":
|
|
return true
|
|
default:
|
|
return false
|
|
}
|
|
}
|
|
```
|
|
|
|
Use `shellFence(f.lang)` in place of the `shellLangs` contains check. Add a `Bash` and a `zsh` fence to `TestCommandTokenForms`.
|
|
|
|
### WR-09: `checkBuiltGo` still accepts Go files `go test ./...` does not build
|
|
|
|
**File:** `internal/docsite/snippet.go:112` (the `ref.Path` passed in), `internal/docsite/snippet.go:174-178` (the segment test), `internal/docsite/snippet.go:179` (`build.ImportDir` on the resolved directory)
|
|
|
|
**Issue:** The new skip list looks only at the lexical `src=` path, and only for `testdata` and a `_` prefix. Two layouts that `go test ./...` never builds still pass `Extract`:
|
|
|
|
- A directory symlink whose name is ordinary. `p/extra` → `p/testdata`, with `hidden.go` calling an undefined function and a test beside it: `Extract(p/extra/hidden.go#Hidden)` returned nil. On that same tree `go list ./...` listed `p` and did not list `p/extra` or `p/testdata`, and `go test ./...` did not run the hidden test. `filepath.WalkDir` does not follow a symlink to a directory, which is why `./...` misses it. The lexical segments of `p/extra/hidden.go` are not `testdata`, and `build.ImportDir` is then pointed at the resolved `testdata` directory, whose own tests make the file look built.
|
|
- A package underneath a `vendor` directory. `vendor/leaf/leaf.go` with a test returned nil from `Extract`, and `go list ./...` did not list it. A package whose own directory is named `vendor` is included by `go test` and must stay allowed; only a `vendor` segment that is not the last directory is skipped.
|
|
|
|
A direct `p/testdata/hidden.go` is refused. The hole is the resolved location and `vendor`.
|
|
|
|
**Fix:** Decide skip rules from the path `./...` walks, and do not let `EvalSymlinks` move the package check into a directory the walk never enters:
|
|
|
|
```go
|
|
func skippedByGoTest(rel string) bool {
|
|
segs := strings.Split(rel, "/")
|
|
dirs := segs[:len(segs)-1]
|
|
for i, seg := range dirs {
|
|
if seg == "testdata" || strings.HasPrefix(seg, "_") {
|
|
return true
|
|
}
|
|
if seg == "vendor" && i != len(dirs)-1 {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|
|
```
|
|
|
|
Before that, `Lstat` each directory prefix of the lexical path and refuse a symlink (`./...` will not descend into it). Call `build.ImportDir` on the lexical directory, not on `filepath.Dir(real)`, so a file symlink is judged as the file the go tool compiles. Add the `p/extra` and `vendor/leaf` cases next to `TestSnippetRootsAndBuild`.
|
|
|
|
### WR-10: A trailing shell continuation is reported as the command name, and the real command on the next line is never checked
|
|
|
|
**File:** `internal/docsite/check_commands.go:146-147`, called once per physical fence line at `internal/docsite/check_commands.go:92-94`
|
|
|
|
**Issue:** `commandWord` returns the first non-flag token after the program. A line `summer \` is therefore the command `\`. Reproduced: an `sh` fence
|
|
|
|
```sh
|
|
summer \
|
|
no:such
|
|
```
|
|
|
|
was reported as `command: "\\" is not a summer or application command` and not as `no:such`. The unknown command sits on the continuation line, which is checked alone and is not a summer invocation, so it passes. The same parse rejects a valid wrap (`summer \` / `docs:build --check`) for the token `\`. `highlightShell` already treats a trailing `\` as a continuation; the checker does not.
|
|
|
|
**Fix:** Join continued lines before `commandWord`, and do not treat `\` as a command word:
|
|
|
|
```go
|
|
var pending string
|
|
for _, line := range f.code {
|
|
text := strings.TrimRight(line.text, " \t")
|
|
if pending != "" {
|
|
text = strings.TrimRight(pending, `\`) + " " + strings.TrimSpace(text)
|
|
pending = ""
|
|
}
|
|
if strings.HasSuffix(text, `\`) {
|
|
pending = text
|
|
continue
|
|
}
|
|
check(d, d.line+line.line, text)
|
|
}
|
|
```
|
|
|
|
### WR-11: The acceptance scanner misses a list-item fence on the marker line, and it marks a normal indented fence as nested
|
|
|
|
**File:** `cmd/summer/phase11_1_acceptance_test.go:490-507` (`stripMarkers`), `cmd/summer/phase11_1_acceptance_test.go:518-520`
|
|
|
|
**Issue:** `stripMarkers` only removes leading spaces and `>` markers. A CommonMark list fence whose opener is on the marker line, `- ```go src=p/p.go#Ok`, does not start with a space or `>`, so `scanDocFences` never records it. The comment on `scanDocFences` says a fence inside a list is visible. It is not, for this form. `Check` does refuse it (reproduced: `snippet: ... src= code block must be a top-level block`), so today's SC4 still fails closed through `docsite.Check`. The independent half of SC4, the nested-`src=` walk and the figcaption count, cannot see it. That is the same blind spot CR-01 had, narrowed to one list shape.
|
|
|
|
The other direction disagrees with the product. Any leading space sets `nested`. Goldmark and `openFence` treat one to three spaces as a top-level fence, caption it, and verify it. SC4 would then fail a page `docs:build --check` accepts, either as a nested `src=` or as a figcaption-count mismatch.
|
|
|
|
**Fix:** Strip a list marker (`- `, `* `, `+ `, or `N. `) as well as `>`, and set `nested` only when a marker was removed or the indent is greater than three spaces. Leave a one-to-three-space opener as top-level, matching `openFence`. Extend `TestAcceptanceFenceScanner` with `- ```go src=...` and with a three-space top-level fence.
|
|
|
|
### Info
|
|
|
|
### IN-09: The identifier index still counts files the default build ignores
|
|
|
|
**File:** `internal/docsite/check_identifiers.go:85-90`
|
|
|
|
**Issue:** `buildIdentIndex` parses every non-test `.go` file under `modules/`. It does not apply build tags, so a declaration that exists only in a `//go:build ignore` file (or a mismatched `_GOOS` file) is `pkg.has` and the span passes without reaching `go doc -c`. The new snippet check refuses that file as a `src=` target; a code span naming the same API does not. This diff does not change the index, and the gap notes it as deferred. It is the identifier-shaped remainder of WR-02.
|
|
|
|
**Fix:** When indexing a directory, keep only the files `build.ImportDir(dir, 0)` lists in `GoFiles` and `CgoFiles`.
|
|
|
|
---
|
|
|
|
_Reviewed: 2026-10-01T07:20:12Z_
|
|
_Reviewer: the agent (gsd-code-reviewer)_
|
|
_Depth: standard_
|