feat(api): /version endpoint with contract-identifier capabilities list - #200
feat(api): /version endpoint with contract-identifier capabilities list#200jaylfc wants to merge 1 commit into
Conversation
Neither a status code nor a version number could answer "does this box actually speak collections". taosmd serve renders the dashboard SPA on unknown non-API paths, so GET /collections returns 200 text/html on a build with no collections code, and checking for a 200 is a check that always passes. Semver does not close the gap either: features land between bumps, and a production box ran a month-stale build unnoticed even though GET /health already reported a version. Add GET /version returning version, commit, commit_source, built_at, built_at_source, and capabilities. Add the same capabilities list to GET /health alongside its existing status and version keys, which are unchanged (taOS and the dashboard consume both). Both endpoints are public by design and join /health in _PUBLIC_PATHS, so monitoring and drift probes keep working on a token-secured box. They expose build identity and capability identifiers only. Capabilities are stable contract identifiers with an explicit version suffix, not feature names: a breaking change to a wire contract becomes collections.v2, so a client pinned to collections.v1 sees the capability disappear rather than collections quietly meaning something new. The list is derived by probing the running build. Each identifier is declared in taosmd/capabilities.py next to the module and symbols that implement it and is advertised only if they resolve, so deleting the code deletes the claim. A divergence test asserts every declared capability's routes exist in the real dispatcher. The commit sha is resolved once and cached, never per request and never by shelling out: git rev-parse in a request path can block on a lock or a slow filesystem. The plumbing is read straight from the filesystem (.git/HEAD to loose ref or packed-refs, including the gitdir: indirection used by worktrees), with an optional packaged taosmd/_build_info.py stamp taking precedence for wheel and container builds. Every step degrades to null rather than raising.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughAdds runtime capability discovery and build metadata resolution, exposes public ChangesVersion and capability discovery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant taosmd.http_server
participant taosmd.capabilities
Client->>taosmd.http_server: GET /version
taosmd.http_server->>taosmd.capabilities: version_payload()
taosmd.capabilities-->>taosmd.http_server: version, build info, capabilities
taosmd.http_server-->>Client: unauthenticated JSON response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/test_version_capabilities.py (1)
183-212: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnused
sourceunpacked in two tests (Ruff RUF059).
test_commit_resolves_from_packed_refs(Line 197) andtest_commit_resolves_detached_head(Line 210) unpackcommit, source = ...but never assert onsource. Static analysis flags both as unused-variable.🧹 Suggested fix
- commit, source = capabilities.resolve_commit(pkg) + commit, _source = capabilities.resolve_commit(pkg)(apply to both occurrences)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_version_capabilities.py` around lines 183 - 212, Update both test_commit_resolves_from_packed_refs and test_commit_resolves_detached_head to avoid unpacking the unused source value from capabilities.resolve_commit; bind only the commit result while preserving the existing commit assertions.Source: Linters/SAST tools
taosmd/capabilities.py (1)
218-229: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueComment says "at import"; behavior is lazy on first call.
The header comment claims commit/build info is "Resolved once, at import, and cached," but
_resolved_build_infois a plain@functools.lru_cache-wrapped function invoked only whenbuild_info()/version_payload()is first called — nothing eagerly resolves it at module import time. This contradicts the accuratebuild_info()docstring below (Line 374: "Resolved once on first call (server startup)"). Worth aligning the wording so a future reader doesn't assume importing this module triggers filesystem/git reads.📝 Suggested wording fix
-# Resolved once, at import, and cached. Two hard rules: +# Resolved once, on first call, and cached thereafter. Two hard rules:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@taosmd/capabilities.py` around lines 218 - 229, Update the build identity header comment above _resolved_build_info to state that commit/build information is resolved once on the first call and then cached, matching the build_info() docstring. Do not change the lazy lru_cache behavior or introduce eager resolution during module import.taosmd/http_server.py (1)
65-80: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value"cacheable" claim isn't backed by response headers.
The docstring calls
/version"cheap, cacheable," but_send_json(used by the/version//healthhandlers) sets noCache-Control/ETag. If "cacheable" is meant as "client-side safe to memoize" that's fine as-is; if it's meant to hint at HTTP/proxy caching, consider adding an explicitCache-Controlheader so the claim matches actual behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@taosmd/http_server.py` around lines 65 - 80, The `/version` documentation describes the response as cacheable without configuring HTTP caching. Update the `/version` response path, using `_send_json`, to emit an explicit appropriate Cache-Control header (and ensure the `/health` behavior remains unchanged unless intentionally shared), or revise the documentation if cacheability is only meant for client-side memoization.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@taosmd/capabilities.py`:
- Around line 218-229: Update the build identity header comment above
_resolved_build_info to state that commit/build information is resolved once on
the first call and then cached, matching the build_info() docstring. Do not
change the lazy lru_cache behavior or introduce eager resolution during module
import.
In `@taosmd/http_server.py`:
- Around line 65-80: The `/version` documentation describes the response as
cacheable without configuring HTTP caching. Update the `/version` response path,
using `_send_json`, to emit an explicit appropriate Cache-Control header (and
ensure the `/health` behavior remains unchanged unless intentionally shared), or
revise the documentation if cacheability is only meant for client-side
memoization.
In `@tests/test_version_capabilities.py`:
- Around line 183-212: Update both test_commit_resolves_from_packed_refs and
test_commit_resolves_detached_head to avoid unpacking the unused source value
from capabilities.resolve_commit; bind only the commit result while preserving
the existing commit assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0f487caf-4fe6-40c9-ad94-46a372a7dac4
📒 Files selected for processing (7)
CHANGELOG.mdREADME.mddocs/collections.mddocs/serve-service.mdtaosmd/capabilities.pytaosmd/http_server.pytests/test_version_capabilities.py
…213) * feat(api): /version endpoint with contract-identifier capabilities list Neither a status code nor a version number could answer "does this box actually speak collections". taosmd serve renders the dashboard SPA on unknown non-API paths, so GET /collections returns 200 text/html on a build with no collections code, and checking for a 200 is a check that always passes. Semver does not close the gap either: features land between bumps, and a production box ran a month-stale build unnoticed even though GET /health already reported a version. Add GET /version returning version, commit, commit_source, built_at, built_at_source, and capabilities. Add the same capabilities list to GET /health alongside its existing status and version keys, which are unchanged (taOS and the dashboard consume both). Both endpoints are public by design and join /health in _PUBLIC_PATHS, so monitoring and drift probes keep working on a token-secured box. They expose build identity and capability identifiers only. Capabilities are stable contract identifiers with an explicit version suffix, not feature names: a breaking change to a wire contract becomes collections.v2, so a client pinned to collections.v1 sees the capability disappear rather than collections quietly meaning something new. The list is derived by probing the running build. Each identifier is declared in taosmd/capabilities.py next to the module and symbols that implement it and is advertised only if they resolve, so deleting the code deletes the claim. A divergence test asserts every declared capability's routes exist in the real dispatcher. The commit sha is resolved once and cached, never per request and never by shelling out: git rev-parse in a request path can block on a lock or a slow filesystem. The plumbing is read straight from the filesystem (.git/HEAD to loose ref or packed-refs, including the gitdir: indirection used by worktrees), with an optional packaged taosmd/_build_info.py stamp taking precedence for wheel and container builds. Every step degrades to null rather than raising. * tsk-ourbp6 [OPEN] Rebase PR #200 (feat/version-capabilities) onto cu * chore: drop the generated uv.lock this branch added taosmd does not track a lockfile. uv sync produced one during the run and it was staged as a side effect, adding 4933 lines to an unrelated change and giving a repo that had not adopted a committed lockfile one by accident. Whether taosmd should track a lockfile is a deliberate decision for its maintainer, not a side effect of an envelope change. The executor now scrubs generated artifacts a branch adds that the base does not track, so this cannot recur.
|
Superseded by #213, now merged: the identical feature content (capabilities.py and tests byte-identical, verified in the #213 review) rebased onto current master. This PR's review history carries over: CodeRabbit's completed pass and the Jay+lead walkthrough approval of 2026-07-26 are referenced in the #213 review comment. Closing per the one-PR-per-task retry rule; branch kept for history. |
HELD FOR REVIEW. Do not merge.
Why
A taOS builder wired an integration to the wrong service and "verified" the route existed by checking an HTTP status code. That check was worthless:
taosmd serverenders the dashboard SPA on unknown non-API paths, soGET /collectionsreturns200 text/htmlon a build that has no collections code at all. The check passes no matter what the server is.Separately, a production Pi ran a month-stale build and nobody noticed, even though
GET /healthalready returns a version. Semver cannot answer "does this box actually speak collections": features land continuously between bumps, so the version number and the feature set are only loosely coupled. A capabilities list answers it directly.taOS will consume this for a drift probe and a user-facing update-alert feature.
What
GET /version(unauthenticated, cheap, cacheable):{ "version": "0.4.0", "commit": "76f72ffef139a9cc08c76d7348b9b25849c845a6", "commit_source": "git", "built_at": "2026-07-21T11:38:52Z", "built_at_source": "install", "capabilities": [ "a2a.v1", "collections.v1", "grants.v1", "graph.v1", "ingest.v1", "search.v1", "shelves.v1", "tasks.v1", "temporal.v1" ] }GET /healthgains the samecapabilitieslist. Its existingstatusandversionkeys are untouched, and a test asserts that old contract explicitly so a future change cannot silently break taOS or the dashboard.Both endpoints join
/healthin_PUBLIC_PATHS. They are public by design: a monitoring or drift probe must keep working on a token-secured box, and the payload is build identity plus capability identifiers only. A test asserts the response key sets are exactly the allowlist and that no path, token, or config value appears, so the public surface cannot grow sensitive fields by accident.The naming contract: why
.v1suffixesThe identifiers are stable contract identifiers, not feature names. The suffix is the entire point.
If the list said
collections, a consumer that checks"collections" in capswould keep passing after the collections wire contract changed underneath it, and would fail at runtime somewhere far from the check. Withcollections.v1, a breaking change to the contract is published ascollections.v2: the consumer pinned tocollections.v1sees the capability disappear, which is a visible, actionable break it can handle at the probe rather than in production. Additive, backwards-compatible changes keep the same identifier, which is exactly when a consumer should not have to care.A build may advertise several versions of one contract at once during a migration window, which gives clients a window to move rather than a flag day.
The right client check is membership, not equality or prefix matching:
How the list is kept honest
The requirement was that it must be impossible for the list to claim a capability the build lacks. A hardcoded constant cannot give that, since it drifts the moment someone deletes code without editing the constant.
So the list is derived by probing the running build.
taosmd/capabilities.pydeclares each identifier as aCapabilityProbebound to the module and the symbols that implement it. A capability is advertised only if the module imports and every symbol resolves. Deleting or renaming an implementation therefore deletes the claim rather than leaving a stale boast.Three tests defend this:
test_capability_is_dropped_when_its_backing_symbol_is_missingdeletes a real backing symbol and assertscollections.v1vanishes from the list while unrelated capabilities survive. This is the anti-drift property, tested rather than asserted in a comment.test_capability_declarations_do_not_diverge_from_the_http_surfacechecks every declared capability's route markers actually appear in the HTTP dispatcher. Adding a capability without wiring its routes, or renaming a route out from under a capability, fails here. This is the divergence test for the one part that cannot be probed at runtime (the handler class is built inside_make_handler, so its routes are not introspectable from module scope).test_capabilities_are_contract_identifiers_with_version_suffixenforces the.vNnaming on every entry, so a bare feature name cannot slip into the contract.The declaration table sits in one place with a comment binding it to the probe mechanism and to the divergence test, per the requirement that a declared constant live adjacent to what it describes.
Commit resolution
Resolved once on first call and cached, so no request pays for it, and it never shells out.
git rev-parsein a request path can block on an index lock, a slow or unmounted filesystem, or a missing git binary, and a monitoring endpoint must not be able to hang. The plumbing we need is plain file reads, so the files are read directly:taosmd/_build_info.pyexportingCOMMIT/BUILT_AT) wins if present, for wheel and container builds..git/HEAD, resolved through a loose ref orpacked-refs, including thegitdir:indirection used by worktrees and submodules (this was verified against a real linked worktree, which is how the example response above was produced).null.Every step is wrapped so a corrupt or partial
.gitdegrades tonullinstead of turning/versioninto a 500.built_atfalls back to the dist-info mtime (install date) when there is no build stamp, since "when did this copy get installed" is what an operator chasing a stale box wants.commit_sourceandbuilt_at_sourcetell the consumer how much to trust the values, which the drift probe needs.Tests cover the loose-ref, packed-refs, detached-HEAD, build-stamp, corrupt-
.git, and not-a-checkout cases. The not-a-checkout test builds its own throwaway directory rather than relying on the dev machine not being a repo.Docs
http_serverendpoint docstring table, the README API table plus a new "Version and capability discovery" section with the client-side check,docs/serve-service.md(public paths and the port-is-live check),docs/collections.md(a "check the server actually speaks collections first" section at the top of the HTTP surface, since that is where integrators look and where the original incident started), and CHANGELOG.Tests
21 new tests in
tests/test_version_capabilities.py. Full suite: 1210 passed, 10 failed.All 10 failures are pre-existing on
origin/masterin this environment, confirmed by running the same files on a cleanorigin/masterworktree with the same venv (identical 10 failures). They are torch/CUDA related on this box (GTX 1050 Ti,sm_61, against a torch build supportingsm_75+), affecting the embedder-backed project-scoping, BM25 cache, reindex, and catalog-pipeline tests. Nothing in this branch touches those paths. 1210 - 21 new = 1189, consistent with the baseline.Summary by CodeRabbit
New Features
GET /versionendpoint with version, build metadata, and supported capabilities.GET /health./versionand/healthaccessible without authentication, including when token security is enabled.Documentation
Bug Fixes
nullwhen unavailable without causing endpoint errors.