API documentation: discovery document and /api docs page omit entire feature surfaces (issues, pulls, releases, checks, social, notifications, review) #272

Closed
opened 2026-09-10 11:04:18 +00:00 by crueber · 3 comments
Owner

What's wrong

The API documentation — both the live discovery document and the static docs page — covers only a fraction of the actual API surface. An integrator reading https://hub.packden.us/api cannot learn that issues, pull requests, releases, checks, stars/watches, notifications, or review threads exist.

Evidence

Live discovery (GET /api/v1, checked 2026-09-09) lists 23 endpoints: me, ssh-keys, owners, repos, imports, mirrors, and the repo-scoped git/api core (refs, resolve, tree, blob, commits, commit, policy, settings, overview, ops, tasks). That's it.

Actually served routes (one server.ExtraRoutes HTTP surface per feature package, all live on both lanes — internal/*/http.go):

Package Surface (not in discovery)
internal/issues issues CRUD, comments, labels, milestones, reactions, attachments (issues/http.go:264-336+)
internal/pulls pulls CRUD, merge, review states
internal/releases releases CRUD, latest, autodraft, assets upload/delete (releases/http.go)
internal/checks checks list/combined/statuses + wct_ CI tokens (checks/http.go:170-233)
internal/review review threads/anchors (review/http.go)
internal/social stars, watches
internal/notify notifications, webhooks
internal/identity access/keys routes
internal/repoimport imports — actually IS in discovery (the one surface that registered)

The imports package is the existence proof that registration works: cmd/walhub/repoimport.go:29-33 calls api.RegisterExposed(repoimport.ExposedTemplates...) at composition, and its routes appear in discovery. The other packages never registered their templates.

The docs page compounds it: web/src/pages/Apidocs.jsx:13-44 carries a hardcoded static route table (policy, settings, ops, tasks, git transport) that has drifted from both discovery and reality — it lacks the same feature surfaces plus newer core routes.

Root cause

No invariant ties "routes actually served" to "routes documented." Each feature package declares its routes in its own Handle/routeX switch (checks/http.go:170, issues/http.go:264, …), but RegisterExposed is opt-in and only imports used it. The docs page is a third, manually-maintained copy. Three sources, zero enforced agreement.

Fix direction

  1. Make registration the norm, not opt-in: every ExtraRoutes package exports its ExposedTemplates (method + template + description + auth class) and composition registers all of them (mirror cmd/walhub/repoimport.go:29-33 — one block per package in the same change). Discovery then derives from the registry, not a hand-list.
  2. Enforce it: a contract test that walks the registered route handlers (or introspects the mux) and fails when a served route is absent from discovery. This is the actual fix — without the test the drift returns with the next feature package.
  3. Docs page: render from discovery (it already fetches it — Apidocs.jsx "renders beside the static route table"); keep the static table only for prose/examples, or delete it once discovery carries descriptions. Include per-route auth requirements and cache classes (the §8 route spec already encodes these).
  4. Per-surface docs worth adding while there: worked examples for the CI workflow (wct_ token → POST status — see the checks-discovery issue), the SSE envelope on task/stream routes, and both-lane URL forms.

Acceptance criteria

  • Discovery lists every repo-scoped and top-level JSON route actually served, with method, template, auth class, and cache class — including issues, pulls, releases, checks, review, social, notifications, identity.
  • A contract test fails CI when a served route is missing from the registry (verified by temporarily removing one).
  • The /api page renders from the live discovery document; the hardcoded static table is gone or demonstrably derived.
  • Each documented route's auth and cache behavior matches what the handler enforces (spot-checked against Handle implementations).
  • SDK methods exist (or links to them) for every documented surface — the SDK is the third client of the same contract; flag any method/route mismatch found.
## What's wrong The API documentation — both the live discovery document and the static docs page — covers only a fraction of the actual API surface. An integrator reading `https://hub.packden.us/api` cannot learn that issues, pull requests, releases, checks, stars/watches, notifications, or review threads exist. ## Evidence **Live discovery (`GET /api/v1`, checked 2026-09-09)** lists 23 endpoints: me, ssh-keys, owners, repos, imports, mirrors, and the repo-scoped git/api core (refs, resolve, tree, blob, commits, commit, policy, settings, overview, ops, tasks). That's it. **Actually served routes** (one `server.ExtraRoutes` HTTP surface per feature package, all live on both lanes — `internal/*/http.go`): | Package | Surface (not in discovery) | |---|---| | `internal/issues` | issues CRUD, comments, labels, milestones, reactions, attachments (issues/http.go:264-336+) | | `internal/pulls` | pulls CRUD, merge, review states | | `internal/releases` | releases CRUD, latest, autodraft, assets upload/delete (releases/http.go) | | `internal/checks` | checks list/combined/statuses + `wct_` CI tokens (checks/http.go:170-233) | | `internal/review` | review threads/anchors (review/http.go) | | `internal/social` | stars, watches | | `internal/notify` | notifications, webhooks | | `internal/identity` | access/keys routes | | `internal/repoimport` | imports — actually IS in discovery (the one surface that registered) | The imports package is the existence proof that registration works: `cmd/walhub/repoimport.go:29-33` calls `api.RegisterExposed(repoimport.ExposedTemplates...)` at composition, and its routes appear in discovery. The other packages never registered their templates. **The docs page compounds it:** `web/src/pages/Apidocs.jsx:13-44` carries a *hardcoded* static route table (policy, settings, ops, tasks, git transport) that has drifted from both discovery and reality — it lacks the same feature surfaces plus newer core routes. ## Root cause No invariant ties "routes actually served" to "routes documented." Each feature package declares its routes in its own `Handle`/`routeX` switch (`checks/http.go:170`, `issues/http.go:264`, …), but `RegisterExposed` is opt-in and only imports used it. The docs page is a third, manually-maintained copy. Three sources, zero enforced agreement. ## Fix direction 1. **Make registration the norm, not opt-in:** every `ExtraRoutes` package exports its `ExposedTemplates` (method + template + description + auth class) and composition registers all of them (mirror `cmd/walhub/repoimport.go:29-33` — one block per package in the same change). Discovery then derives from the registry, not a hand-list. 2. **Enforce it:** a contract test that walks the registered route handlers (or introspects the mux) and fails when a served route is absent from discovery. This is the actual fix — without the test the drift returns with the next feature package. 3. **Docs page:** render from discovery (it already fetches it — `Apidocs.jsx` "renders beside the static route table"); keep the static table only for prose/examples, or delete it once discovery carries descriptions. Include per-route auth requirements and cache classes (the §8 route spec already encodes these). 4. **Per-surface docs worth adding while there:** worked examples for the CI workflow (`wct_` token → POST status — see the checks-discovery issue), the SSE envelope on task/stream routes, and both-lane URL forms. ## Acceptance criteria - [ ] Discovery lists every repo-scoped and top-level JSON route actually served, with method, template, auth class, and cache class — including issues, pulls, releases, checks, review, social, notifications, identity. - [ ] A contract test fails CI when a served route is missing from the registry (verified by temporarily removing one). - [ ] The `/api` page renders from the live discovery document; the hardcoded static table is gone or demonstrably derived. - [ ] Each documented route's auth and cache behavior matches what the handler enforces (spot-checked against `Handle` implementations). - [ ] SDK methods exist (or links to them) for every documented surface — the SDK is the third client of the same contract; flag any method/route mismatch found.
Author
Owner

Fix ready for review: PR #294 (fix/issue-272) — ExposedTemplates + registration for issues, pulls, releases, review, social, notify, identity, tags, and mirror repo lanes; per-surface contract tests + composition test (drift fails CI, verified by temporary removal); /api page extended with derived route table, per-surface auth/shapes, and release-upload + notification-SSE examples; docs in 07_api.md + 14_extensibility.md. No behavior change, no new deps. Not merging — awaiting review.

Fix ready for review: PR #294 (fix/issue-272) — ExposedTemplates + registration for issues, pulls, releases, review, social, notify, identity, tags, and mirror repo lanes; per-surface contract tests + composition test (drift fails CI, verified by temporary removal); /api page extended with derived route table, per-surface auth/shapes, and release-upload + notification-SSE examples; docs in 07_api.md + 14_extensibility.md. No behavior change, no new deps. Not merging — awaiting review.
Author
Owner

Review of PR 294 (fix/issue-272, commit 0c612df): template<->implementation spot-checks (identity teams/members PUT|DELETE owner-gated, issues lanes/methods, releases api-lane 5 shapes + HandleRepo bytes excluded, mirror top-level + repo lanes, notify watch/webhooks/stream gates, pulls + review routes) all match; both lanes collapse to one template everywhere; byte routes (release asset bytes, attachment bytes via ChainRepo) correctly excluded as static contract. Composition test is genuine (runs before buildCollab in package order, no TestMain; fails in isolation and full suite if a registration drops; mirror/checks wiring pre-existed and untouched). newIdentity/newIssues constructors move existing code verbatim + render-deduped RegisterExposed only, no RegisterKind, no behavior change. Coverage: identity 97.2, issues 96.3, mirror 96.9, notify 95.8, pulls 97.7, releases 99.8, review 95.9, social 99.5, tags 97.9 (gate is internal-only; cmd/walhub exempt). gofmt/vet clean; 557 node tests pass; no go.mod/package.json changes. Two small fixes pushed as 37df9a2: (1) Apidocs mirror sync GET row said (write) but syncStatus is open read like GET mirror; (2) 14_extensibility Wave C2/Feature 06/Feature 07 no-discovery sentences + #271 stay-unlisted tail now explicitly superseded by #272 (law 12). Note, no browser drive (text-only surfaces; vite build validates the JSX). Caveat: Apidocs static table itself has no CI drift check (TestExposedCoversRoutes pins template<->route, not table<->template) despite the drift-fails-CI wording; acceptable since the page also renders live discovery. Recommendation: ready to merge.

Review of PR 294 (fix/issue-272, commit 0c612df): template<->implementation spot-checks (identity teams/members PUT|DELETE owner-gated, issues lanes/methods, releases api-lane 5 shapes + HandleRepo bytes excluded, mirror top-level + repo lanes, notify watch/webhooks/stream gates, pulls + review routes) all match; both lanes collapse to one template everywhere; byte routes (release asset bytes, attachment bytes via ChainRepo) correctly excluded as static contract. Composition test is genuine (runs before buildCollab in package order, no TestMain; fails in isolation and full suite if a registration drops; mirror/checks wiring pre-existed and untouched). newIdentity/newIssues constructors move existing code verbatim + render-deduped RegisterExposed only, no RegisterKind, no behavior change. Coverage: identity 97.2, issues 96.3, mirror 96.9, notify 95.8, pulls 97.7, releases 99.8, review 95.9, social 99.5, tags 97.9 (gate is internal-only; cmd/walhub exempt). gofmt/vet clean; 557 node tests pass; no go.mod/package.json changes. Two small fixes pushed as 37df9a2: (1) Apidocs mirror sync GET row said (write) but syncStatus is open read like GET mirror; (2) 14_extensibility Wave C2/Feature 06/Feature 07 no-discovery sentences + #271 stay-unlisted tail now explicitly superseded by #272 (law 12). Note, no browser drive (text-only surfaces; vite build validates the JSX). Caveat: Apidocs static table itself has no CI drift check (TestExposedCoversRoutes pins template<->route, not table<->template) despite the drift-fails-CI wording; acceptable since the page also renders live discovery. Recommendation: ready to merge.
Author
Owner

Fixed by PR #294 incl. review doc/auth fixes (all 8 surfaces exposed + tested; gates green), merged. Closing.

Fixed by PR #294 incl. review doc/auth fixes (all 8 surfaces exposed + tested; gates green), merged. Closing.
crueber added this to the v1 milestone 2026-09-10 22:27:09 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
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
crueber/walhub#272
No description provided.