Repository navigation
Conversation
This was referenced Oct 29, 2026
Pin the current wire format of GET /api/search, /api/search/status,
/api/pages/{id}/links and /api/links/broken through the real router, so
the following refactors can be proven byte-identical.
The links fixture uses a real auth service with an admin and an editor,
because RequireAdmin always answers 403 with authentication disabled and
would otherwise hide the 200 case.
SQLiteIndex.Search left Items nil for a text query without a match, so GET /api/search serialized "items": null. The early returns and the tag-only path already send an empty list; make the text path consistent. Behavior-Change: GET /api/search now returns "items": [] instead of "items": null when a text query matches nothing.
Both slices carried their own copy of the localized error envelope, byte-identical to dto.ErrorResponse. Write the shared type and delete the copies, as the tags slice already does. The helper names and the wire format stay unchanged.
The search status, search, link status and broken links handlers sent core types straight to the client. Map them to dto types first so the API docs can describe them without exposing core structs. The output is byte-identical: backlinks and broken_incoming keep sending null for an empty list, a follow-up changes that.
ToLinkStatus kept nil backlinks and broken_incoming, so
GET /api/pages/{id}/links serialized "backlinks": null and
"broken_incoming": null for a page nothing links to. Outgoing lists
already send []. Send [] for all four lists, which the generated API
docs mark as required.
Behavior-Change: GET /api/pages/{id}/links now returns [] instead of null for backlinks and broken_incoming when a page has none.
Annotate GET /api/search, /api/search/status, /api/pages/{id}/links and
/api/links/broken, regenerate docs/api, and drop their entries from the
route-coverage allowlist.
assertOperation learns object 200 responses and path parameters; the new
shape tests also pin the nested dto models. README and ADR 0014 no longer
list the search and links core types as undocumented.
zorak1103
force-pushed
the
docs/api-search-links
branch
from
November 2, 2026 17:20
359c56f to
c212d69
Compare
zorak1103
marked this pull request as ready for review
November 2, 2026 18:02
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue
Part of #1661. This is PR 4, "search, links". It follows the pattern from #1682 and #1741.
What changed
This documents the 4 remaining read-only routes of
internal/wiki/searchandinternal/wiki/links:GET /api/search,GET /api/search/status,GET /api/pages/{id}/linksandGET /api/links/broken. It removes them from the allowlist ininternal/http/api_docs_allowlist_test.go. Routes, status codes and JSON keys are unchanged. The only wire changes are the twonullto[]fixes below, each in its own commit with aBehavior-Change:trailer.The PR has 6 commits:
test(wiki): route tests for both packages, run through the real router, pinning today's wire format (shape, query semantics, exact error bytes, 401 and 200).GET /api/links/brokenis tested with a real auth service with an admin and an editor, becauseRequireAdminalways answers 403 when authentication is disabled and would hide the 200 case. All of these tests pass against the old code.fix(search):GET /api/searchsends"items": []instead of"items": nullwhen a text query matches nothing. The early returns and the tag-only path already sent[]. The UI already tolerates both (data.items || []).Behavior-Change:trailer.refactor(wiki): the package-local error helpers insearch/errors.goandlinks/errors.gomove todto.ErrorResponse/dto.ErrorDetail, asdocs/api/README.mdasks. The bytes are identical and the helper names stay.refactor(wiki): the four handlers serializedtotypes through new mappers (dto.ToSearchResult,ToIndexingStatus,ToLinkStatus,ToBrokenLinks) instead of core types. This is byte-identical, which the commit-1 tests and marshal-parity tests indtoshow.fix(links):GET /api/pages/{id}/linkssends[]instead ofnullforbacklinksandbroken_incomingof a page without links (outgoingsandbroken_outgoingsalready did). The generated spec marks all four lists as required, and the README says to normalize nil slices in required fields. The UI already usesstatus?.backlinks ?? [].Behavior-Change:trailer.docs(api): swag annotations, regenerateddocs/api, the four allowlist entries removed, README and ADR 0014 updated.ErrorResponsewith the codes named. 401, 403, 429 and 503 areLegacyError. 500 is a broadobjectwhere the handler can fail, because the handler and the auth middleware return different shapes (same workaround as tags)./search/statusdocuments no 400 and aLegacyError500 only, because its handler cannot fail./links/brokenis documented as admin only, also with public access on. API keys act with the lesser of owner and key role, and with authentication disabled the route answers 403.assertOperationaccepts an object 200 response (okRef/okProps) and path parameters, and a nil parameter map now means "none". The existing tags and properties tests are unchanged. The newTestAPIDocs_Search_DescribeReadOperationsandTestAPIDocs_Links_DescribeOperationsalso pin the nested models, and the ten new models are indocumentedModels.Inconsistencies found (for the list in #1661, not fixed here)
limit/offsetdiffer by path. Text search hands them to SQLite:limit=-1returns all rows,limit=0returns no items, and there is no upper bound. Tag-only search turnslimit <= 0into 20 and clampsoffset. Tags and properties clamp at 200.qis required", althoughtagsalone is enough.{"links": ...}, while other lists are bare arrays.finished_aton/api/search/statusis0001-01-01T00:00:00Zuntil the first index run finishes.search_unavailableandlink_service_unavailablepaths are unreachable in a real Wiki:initSearchandinitLinkServiceabort construction on error, so neither dependency can be nil. They are left in place.How I tested it
go build ./...andgo testpass for./internal,./internal/http/...,./internal/wiki/search,./internal/wiki/links,./internal/searchand./internal/links.make api-docsfollowed bygit diff --exit-code -- docs/apishows no drift.make verify-commits BASE=origin/mainpasses.golangci-lintfinds nothing in the touched packages.GOOS=linuxandGOOS=darwingo vetpass for the touched packages.test-go-windowsjob failed ininternal/restore, a package this PR does not touch. It is a race in the test, fixed in test(restore): wait for the upload cleanup before checking the temp file #1764. Once that is merged, this PR needs a rebase or a re-run of the job.make verifyas a whole (its recipe fails on a Windows path quirk; I ranscripts/check-private-references.shdirectly) and the E2E suite.broken_incomingwith a non-empty list (it needs a deleted or moved target page; it shares its item type withbacklinks, whose keys are pinned), and a nil status inToIndexingStatus(unreachable in a real Wiki, pinned in thedtotest).Checklist
if featurebranches in shared code (rule 1)Behavior-Change:trailer (commit style)