tsk-ourbp6 [OPEN] Rebase PR #200 (feat/version-capabilities) onto cu - #213
Merged
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.
📝 WalkthroughWalkthroughAdds runtime capability and build-identity reporting, exposes public ChangesCapability Discovery and Public Version Reporting
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
|
PR Summary by Qodofeat(api): Add /version endpoint with runtime capability advertisement
AI Description
Diagram
High-Level Assessment
Files changed (8)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
13 rules 1. Unvalidated build stamp
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+231
to
+250
| def _build_stamp() -> dict | None: | ||
| """Optional build-time stamp written by a packaging step. | ||
|
|
||
| A wheel or container build may drop a ``taosmd/_build_info.py`` exporting | ||
| ``COMMIT`` and/or ``BUILT_AT`` (ISO 8601). It is absent from a plain source | ||
| checkout, which is why the git reader below exists. | ||
| """ | ||
| try: | ||
| module = importlib.import_module("taosmd._build_info") | ||
| except Exception: # noqa: BLE001 - unstamped build is the normal case | ||
| return None | ||
| commit = getattr(module, "COMMIT", None) | ||
| built_at = getattr(module, "BUILT_AT", None) | ||
| if not isinstance(commit, str) or not commit.strip(): | ||
| commit = None | ||
| if not isinstance(built_at, str) or not built_at.strip(): | ||
| built_at = None | ||
| if commit is None and built_at is None: | ||
| return None | ||
| return {"commit": commit, "built_at": built_at} |
There was a problem hiding this comment.
1. Unvalidated build stamp 🐞 Bug ≡ Correctness
taosmd.capabilities._build_stamp() accepts any non-empty COMMIT/BUILT_AT strings from taosmd/_build_info.py, and /version will publish them as-is. This can violate the documented contract (commit must be a 40-hex sha; built_at must be ISO 8601 UTC) and break clients/monitoring that rely on those shapes.
Agent Prompt
## Issue description
`_build_stamp()` currently treats any non-empty `COMMIT` and `BUILT_AT` values as valid and returns them unchanged. Because `resolve_commit()` / `_resolve_built_at()` prefer the build stamp, `/version` can return malformed values (e.g., short sha, whitespace-padded sha, non-ISO timestamp), violating the public contract documented in the README.
## Issue Context
Git-derived commits are already validated via `_looks_like_sha()`, and install-derived `built_at` is formatted as `...Z`. Only the build-stamp path lacks validation/normalization.
## Fix Focus Areas
- taosmd/capabilities.py[231-250]
- taosmd/capabilities.py[307-333]
- taosmd/capabilities.py[335-356]
## Suggested fix
1. In `_build_stamp()`:
- `commit = commit.strip()` and validate with `_looks_like_sha(commit)`; otherwise set `commit = None`.
- `built_at = built_at.strip()` and strictly parse/validate ISO-8601; require UTC (either `Z` or `+00:00`), then normalize to `YYYY-MM-DDTHH:MM:SSZ`. If invalid, set `built_at = None`.
2. Keep the existing fallback behavior: if stamp values are invalid/None, fall back to git plumbing and/or install mtime.
3. Add/extend tests to cover malformed stamp values (short sha, non-hex, whitespace, non-UTC timestamps) and assert they become `null` rather than being published.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@taosmd/http_server.py`:
- Line 624: Update the `_check_token` docstring to mention `/version` alongside
the existing public health and UI paths, matching the `_PUBLIC_PATHS` allowlist
without changing authentication behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9ecd77bc-a5ca-4358-8a70-0cc50b0dd1fc
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
CHANGELOG.mdREADME.mddocs/collections.mddocs/serve-service.mdtaosmd/capabilities.pytaosmd/http_server.pytests/test_version_capabilities.py
|
|
||
| # Paths that are always public regardless of the token setting. | ||
| _PUBLIC_PATHS = frozenset({"/", "/ui", "/health"}) | ||
| _PUBLIC_PATHS = frozenset({"/", "/ui", "/health", "/version"}) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
_check_token docstring now stale.
_PUBLIC_PATHS gains "/version" here, but the nearby _check_token docstring ("Public paths (health, UI) are always authorised.") still only names health/UI, not version. Worth a one-line update so the docstring doesn't mislead a future reader of the auth gate.
📝 Suggested docstring tweak
- Public paths (health, UI) are always authorised.
+ Public paths (health, version, UI) are always authorised.🤖 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` at line 624, Update the `_check_token` docstring to
mention `/version` alongside the existing public health and UI paths, matching
the `_PUBLIC_PATHS` allowlist without changing authentication behavior.
Owner
Author
|
Lead review with verification evidence (the PR body omits it): Content verified as the approved #200 feature, correctly rebased. Process note: the card said force-push One blocker, same as #212: remove |
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.
This was referenced Jul 27, 2026
jaylfc
added a commit
that referenced
this pull request
Aug 17, 2026
… never exercised their own rule (#311) Replaces the all-or-nothing #212 revert with eight per-rule reverts, one per validation rule, so a missing sub-rule can be seen rather than masked by the other seven. That granularity paid: R1 and R5 were false negatives. The test inputs used the string "not a list", which has len 10 and so tripped the max-refs guard, and which iterates into characters and so tripped the item-type guard, before the isinstance check was ever reached. Disabling the is-a-list rule left both tests green. The inputs are now non-iterable (42), and the same perturbation turns both red. Relabels the report TARGETED/UNRELATED, reserving DECORATIVE for tautologies (none found), which removes the contradiction where one test was REAL in one table and DECORATIVE in another. Splits row 13 out: its status assertion is duplicated at the service layer, so only its message assertion is load-bearing. Keeps the test_capability_declarations_do_not_diverge retraction prominent. Reviewer's note on provenance: the PR body's file table and its reported suite count of 1401 belong to a different tree. They credit this PR with scripts/normalise_handle_gate.py, scripts/check_deleted_symbols.py, tests/test_deleted_symbols.py, tests/test_normalise_handle_gate.py and a changelog entry for card tsk-vcda2i, all of which are #306's and #308's already-merged work. This change is two files, 308 insertions, and the suite on the trial merge is 1435 passed / 12 skipped. Card tsk-24ybw5. Review: #311 (comment)
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
1 participant
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.
Autonomous build of board card tsk-ourbp6.
Files:
README.md | 46 +-
docs/collections.md | 33 +
docs/serve-service.md | 23 +-
taosmd/capabilities.py | 390 +++
taosmd/http_server.py | 38 +-
tests/test_version_capabilities.py | 388 +++
uv.lock | 4933 ++++++++++++++++++++++++++++++++++++
8 files changed, 5841 insertions(+), 13 deletions(-)
Summary by CodeRabbit
New Features
GET /versionendpoint with build metadata and supported capability identifiers.GET /healthresponses to include capabilities.Documentation