Skip to content

docs: propagate /v1 API versioning + fix stale counts and framing#162

Merged
surpradhan merged 4 commits into
mainfrom
docs/v1-and-staleness-sweep
Jul 19, 2026
Merged

docs: propagate /v1 API versioning + fix stale counts and framing#162
surpradhan merged 4 commits into
mainfrom
docs/v1-and-staleness-sweep

Conversation

@surpradhan

@surpradhan surpradhan commented Jul 19, 2026

Copy link
Copy Markdown
Owner

What does this PR do?

Brings four docs in line with shipped reality:

  • /v1 API versioning (shipped 2026-06-16, previously documented only in openapi.json): README, SETUP §14, OPERATIONS, and QUICK_REFERENCE now note the /v1 prefix + API-Version: 1 header, backward-compatible unversioned paths, and unversioned infra endpoints.
  • Stale counts: README badge Tests: 200+700+; 82 tests (55+27) → resilient 700+ phrasing (measured: 494 unit + 264 integration at time of writing); Python block 118 → 300+.
  • SETUP /health example showed "version": "0.2.0" — the server reports the package build 1.0.0 (verified against src/server.js); header now separates protocol vs server version.
  • OPERATIONS intro reframed from "Covers Phase 13 (Hosted SaaS)" — a phase framing the project retired — to neutral self-hosted production operations; adds the missing §6/§7 TOC entries.
  • Roadmap note: Phases 13–17 marked built-but-parked, referencing the existing direction note.

Deliberately unchanged: QUICK_REFERENCE's scopes table (reported as missing an admin scope — false alarm: no such API-key scope exists; admin auth is the separate ADMIN_TOKEN bearer, per src/server.js validScopes).

Why?

CTO review Wave 1, item 7 — the API's versioning story existed only in code and openapi.json, and multiple stale claims (counts, versions, phase framing) contradicted the repo.

How to test?

Every number and path is verifiable in-repo; test counts reproducible via node --test tests/unit/*.test.js / tests/integration/*.test.js.

Checklist

  • Docs only
  • All claims verified against source

🤖 Generated with Claude Code

- README: /v1 note in API section (unversioned paths remain supported;
  infra endpoints unversioned), lead examples on /v1; Tests badge 200+ →
  700+; testing section 82 → 700+ with resilient phrasing; Python SDK
  block 118 → 300+; roadmap notes Phases 13-17 built-but-parked
- SETUP: /health example reports server build 1.0.0 (was 0.2.0) + checks
  field; /v1 note in §14; header disambiguates protocol vs server version
- OPERATIONS: intro reframed from 'Phase 13 (Hosted SaaS)' to self-hosted
  production operations; missing TOC entries for §6/§7; /v1 note
- QUICK_REFERENCE: /v1 note + footer date

All numbers measured (494 unit + 255 integration Node at time of writing;
301 Python collected); scopes table left unchanged — there is no 'admin'
API-key scope (admin uses ADMIN_TOKEN), verified against src/server.js.

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

@surpradhan surpradhan left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent validation report (pr-validator)

Verdict: Approve. Docs-only PR; every load-bearing claim was probed empirically against the branch worktree (09b28fa). Zero actionable items; one optional nit.

Scope

gh pr diff 162 --name-only = exactly README.md, SETUP.md, OPERATIONS.md, docs/QUICK_REFERENCE.md — matches the stated docs-only scope. Worktree clean and identical to the PR head.

Live server probes (booted from the branch, temp DB, ephemeral port)

  • GET /health{"ok":true,"service":"aep-ingest","version":"1.0.0","checks":{"db":"ok"}}byte-identical to SETUP's updated example. SERVER_VERSION traced to package.json 1.0.0 (src/server.js:6), confirming the 0.2.0→1.0.0 fix.
  • GET /v1/sessions no auth (with DASHBOARD_TOKEN set) → 401 + API-Version: 1 header. GET /sessions → same 401 + header (backward-compat path live).
  • Dev-mode nuance verified too: with DASHBOARD_TOKEN unset, unauth reads return 200 (per src/auth.js fail-open-in-dev) — consistent with existing docs, not contradicted by this PR.
  • GET /v1/health404 and POST /v1/events no auth → 401 — matches the note that infra probes are unversioned while consumer endpoints are.
  • Source cross-check: API-Version header set globally (src/server.js:197); v1 router mounted at /v1 and / (:1904-1908); /health, /ready, /metrics/prometheus, /dashboard, /docs, /openapi.json, /admin/* all app-level (unversioned) — every endpoint the notes classify is classified correctly, incl. the subtle /metrics (versioned, v1.get) vs /metrics/prometheus (unversioned) split.

Test-count claims (measured on this branch)

  • npm run test:unit494 pass / 0 fail; npm run test:integration264 pass / 0 fail → 758 total. README's badge 700+ and "700+ (490+ unit + 250+ integration)" hold, and the resilient phrasing already absorbed drift since authoring (255→264 integration).
  • Python SDK: pytest collect → 281 unit + 20 integration = 301 — "300+ (280+ unit + 20 integration)" holds exactly.
  • npm run lint → clean.

Staleness sweep of the four files

  • No leftover 82 tests, 200+, <PORT>; all remaining 0.2.0 mentions are protocol-spec (specversion), which is correct — SETUP/QUICK_REFERENCE headers now cleanly separate protocol vs server version.
  • validScopes = ["read","write"] (src/server.js:1674) confirms the deliberate non-change to the scopes table (no admin API-key scope exists).

Markdown sanity

Programmatic anchor check over OPERATIONS.md: all internal TOC links resolve, including the new #6-webhooks--alerts-phase-16 / #7-export--cold-storage-phase-17 (consistent with the existing #3-projects-tiers--quotas style).

CI

gh pr checks 16214/14 pass (incl. Required checks in sync).

Findings

  • Nit (optional, non-blocking)OPERATIONS.md:486: one residual "Phase 13 (Hosted SaaS groundwork) is complete" in §5's body. The intro reframe (the PR's stated scope) is done; this line is historical context inside a section still titled "Phase 13 production checklist", so it reads as intentional history rather than framing. Fine to leave or sweep in a follow-up.

Validation environment cleaned up (node_modules symlink removed, temp DB/logs deleted, worktree clean).

@surpradhan surpradhan left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent code review — PR #162 (docs: propagate /v1 API versioning + fix stale counts and framing)

Reviewed against source on the rebased branch (09b28fa, parent = current main cdc478f), with a live-server probe and full local gate run.

Verification performed

  • /v1 claims vs src/server.js: verified the v1 router is mounted at both /v1 and / (lines 1905/1908), the API-Version: 1 header middleware is global (line 197), and infra/UI/admin routes stay on app (lines 203–204, 310, 335, 1343, 1667+). Empirically probed a live server (scratch DB, port 8899, cleaned up): /v1/sessions→200, /sessions→200, /v1/health→404, /v1/admin/keys→404, /v1/metrics/prometheus→404, and API-Version: 1 present on every response including /health, /admin/*, and 404s. All four docs' versioning notes match observed behavior.
  • openapi.json as spec: confirmed the servers block lists /v1 as recommended + root as legacy, with per-path root-only overrides for infra probes (e.g. /health) — consistent with the doc notes.
  • Consistency with in-flight PR #161 (VERSIONING.md): README's unversioned list (/health, /ready, /metrics/prometheus, /admin/*, /dashboard, /docs, /openapi.json) matches #161's list exactly. No contradiction between the two in-flight docs.
  • SETUP /health example: byte-accurate vs the live handler: {"ok":true,"service":"aep-ingest","version":"1.0.0","checks":{"db":"ok"}} (SERVER_VERSION = package.json 1.0.0). The protocol-vs-build disambiguation in the SETUP header is correct.
  • Test counts on THIS branch post-rebase: Node node --test: 494 unit + 264 integration = 758, all pass → badge 700+, 490+ unit, 250+ integration all hold (integration grew 255→264 after the rebase over #157's globbed runner; the resilient phrasing absorbs it). Python pytest --collect-only: 281 unit + 20 integration = 301300+ (280+ unit + 20 integration) is exactly right. Integration tests do cover /v1 (server.test.js:4439 "API versioning — /v1 prefix and API-Version header"), so the README parenthetical is accurate.
  • OPERATIONS reframing: §5 "Phase 13 production checklist" heading + body ("Phase 13 … is complete") read as historical provenance, same as §6 "(Phase 16)" / §7 "(Phase 17)" — no contradiction with the neutral intro or the README "built but parked" note. New TOC anchors #6-webhooks--alerts-phase-16 / #7-export--cold-storage-phase-17 match GitHub's anchor generation for the existing headings.
  • Rebase artifacts: none — no conflict markers, each versioning note appears exactly once per file, no duplication against #156/#159's README changes.
  • PR-body "deliberately unchanged" claim: correct — src/server.js:1674 validScopes = ["read", "write"]; no admin API-key scope exists.
  • Gates: npm run lint clean; 494u + 264i pass locally; all 14 CI checks green.

Findings

Nit 1 — README.md ~332–337 (API Response Formats): mixed path styles within one list. The 202 and first 200 example were updated to POST /v1/events / GET /v1/sessions, but the next entries in the same list stay unversioned (GET /sessions/{sessionId}/events, GET /sessions/{sessionId}/audit-bundle, GET /workflows/{traceId}/audit-bundle). Both work (note explains equivalence), but pick one style — suggest converting the remaining three to /v1 to match the "recommended" framing.

Nit 2 — SETUP.md ~536 (versioning note): the unversioned carve-out omits /openapi.json, which README's note and #161's VERSIONING.md both list. Suggest: "…the dashboard/docs UI (/dashboard, /docs, /openapi.json), and /admin/* are not versioned." Keeps the two in-flight docs and README saying the same thing.

Nit 3 — OPERATIONS.md ~22–25 (versioning note): "…is also served under the /v1 prefix with an API-Version: 1 response header, while /admin/* and the infra probes … are unversioned" reads as if the header is specific to /v1-served endpoints. Verified live: the header is set by global middleware on every response, including /admin/* and the infra probes. The other three docs say "every response includes an API-Version: 1 header" — align OPERATIONS with that, e.g. "…is also served under the /v1 prefix (all responses carry an API-Version: 1 header), while /admin/* and the infra probes … are unversioned."

Non-blocking observation (PR body only, not the diff): the body says "measured: 494 unit + 255 integration at time of writing" — post-rebase it's 264 integration. The doc text's 250+ phrasing is unaffected; optionally refresh the body number on the next push.

What looks good

  • Every factual claim in the diff checks out against source or live behavior — including the /health shape, the /v1 routing matrix, the header semantics, and every count (Node 758, Python 301, Go 87 vs 80+).
  • The resilient "N+" phrasing is the right call for counts that CI keeps growing.
  • The scopes-table "false alarm" judgment (leaving QUICK_REFERENCE unchanged) is correct and well-reasoned.
  • Unversioned-endpoint lists are consistent with the in-flight VERSIONING.md (#161), so the two PRs can land in either order without contradicting each other.

Verdict: Approve (3 nits above; no Critical or Suggestion-severity findings)

Independent review session — reviewer is not the author and will not merge.

…lobal header wording, Phase-13 history phrasing

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

Copy link
Copy Markdown
Owner Author

All four round-1 nits applied in 8735401: README's remaining three response-format examples converted to /v1 (consistent style); SETUP's carve-out now names /dashboard, /docs, /openapi.json; OPERATIONS reworded so the API-Version header reads as global, not /v1-specific; §5's residual 'Phase 13 (Hosted SaaS groundwork) is complete' recast as infrastructure history with the checklist framed as current for any self-hosted deployment. PR body count refreshed (264 integration post-rebase).

@surpradhan surpradhan left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validator round 2 — delta 09b28fa..8735401 (final validation)

Verdict: Approve. All four round-1 findings are fixed; no new issues introduced.

Delta scope

git diff 09b28fa..8735401 touches only README.md (4 lines), SETUP.md (2 lines), OPERATIONS.md (11 lines) — exactly the four requested changes, nothing else:

  1. README §API Response Formats — the two remaining unversioned example headers (GET /sessions/{sessionId}/events, GET /sessions/{sessionId}/audit-bundle + GET /workflows/{traceId}/audit-bundle) now use /v1. All five endpoint examples in the section are now consistently /v1; the 400/401/403 examples name no endpoints. ✅
  2. SETUP.md carve-out — dashboard/docs UI exclusion now enumerates /dashboard, /docs, /openapi.json. ✅
  3. OPERATIONS.md header wording — decouples the API-Version: 1 header from the /v1 prefix ('all responses carry'). ✅ and more accurate than before: the header is set by a global app.use in src/server.js:197 registered ahead of all routes.
  4. OPERATIONS.md §5 Phase-13 line (my round-1 nit) — reframed as shipped infrastructure + still-current self-hosted checklist. ✅

Empirical probes (live server boot from the PR worktree)

  • POST /v1/events202 {"accepted":true,"duplicate":false,"id":"evt_..."} (matches README example byte-for-shape)
  • GET /v1/sessions → 200, GET /v1/sessions/{id}/events → 200, GET /v1/sessions/{id}/audit-bundle → 200, GET /v1/workflows/{traceId}/audit-bundle → 200 (with AUDIT_SIGNING_SECRET set)
  • API-Version: 1 header confirmed on /health, /dashboard, and /v1/sessions — the strengthened 'all responses' wording holds, including on unversioned infra/UI routes.
  • Note: src/openapi.json intentionally documents only the unversioned canonical paths (0 /v1 entries, per PR #141 design); route existence verified against src/server.js v1 router (mounted at both /v1 and /) and live HTTP.

CI

All 14/14 checks green on head 8735401 (Operator unit tests, Postgres parity, Node 20/22 SDK, Python 3.10–3.13, Go SDK, otelbridge, Required checks in sync, build, docker, test 20.x).

Zero actionable items remaining. Docs-only change, head matches validated worktree, working tree clean.

@surpradhan surpradhan left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round-2 review (delta 09b28fa..8735401) — code-reviewer

Verified each of the four round-1/validator findings against the delta commit 8735401 and the server source:

  1. README mixed /v1 example styles — FIXED. The response-structure examples block (README.md lines 321–336) is now uniformly /v1: POST /v1/events, GET /v1/sessions, GET /v1/sessions/{sessionId}/events, and both audit-bundle examples. Remaining unversioned root paths (quickstart POST /admin/keys, the architecture diagram's POST /events) are correct as-is — admin is genuinely unversioned and the versioning note explicitly documents root-path support.
  2. SETUP carve-out missing /openapi.json — FIXED. The versioning note now lists /dashboard, /docs, /openapi.json explicitly. Verified against src/server.js: all three are mounted directly on app (lines 359/369/399), not on the v1 router, so the carve-out is code-accurate.
  3. OPERATIONS header wording implying API-Version is /v1-specific — FIXED. Now reads "all responses carry an API-Version: 1 header." Verified: the header middleware (src/server.js:197) is a bare app.use registered before every route registration (first route is /health at line 310), so admin, infra probes, dashboard, and both versioned/unversioned consumer routes all get the header. The claim is accurate, not an overclaim.
  4. Residual "Phase 13 (Hosted SaaS groundwork) is complete" in §5 body — FIXED. Reworded to "The Phase 13 infrastructure shipped in full — … — and this checklist remains current for any self-hosted production deployment." This does not overclaim: it drops the SaaS framing, the PR-B–E enumeration is factual (all merged), and the checklist items themselves are untouched and remain true (verified spot-checks: v2-only signature enforcement, QUOTA_ENFORCEMENT, npm run prune all match source).

No new inaccuracies introduced. Delta touches only OPERATIONS.md/README.md/SETUP.md (9+/8−); files are NUL-clean UTF-8; no trailing-whitespace or formatting regressions.

Gates: npm run lint clean; npm test 264/264 pass (unit + integration).

Zero actionable items. Verdict: Approve.

(Posted as comment — reviewer shares the author's gh identity, so a formal Approve review is not possible; treat this as the round-2 APPROVE.)

@surpradhan
surpradhan merged commit 353af48 into main Jul 19, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant