feat: add provenance-bound retrieval reference foundation - #487
100yenadmin wants to merge 2 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf44dc8198
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| owner_rebuild_needed = ( | ||
| _fts_needs_rebuild_structural(conn, spec) if winner_state_needs_repair else False | ||
| ) | ||
| if not owner_rebuild_needed and not structural_repair_needed and deep_repair_needed: |
There was a problem hiding this comment.
When an FTS trigger has disappeared and content was subsequently updated without changing the row count, structural_repair_needed is true solely because of the missing trigger while _fts_needs_rebuild_structural remains false. This guard then skips the only deep integrity check, so an explicit doctor repair merely recreates the trigger, clears the corruption marker, and leaves the already-stale index malformed; the deep check must still run for this combination.
Useful? React with 👍 / 👎.
| SELECT target_json, revision_json | ||
| FROM {RETRIEVAL_REFERENCES_TABLE} | ||
| WHERE token_digest = ? |
There was a problem hiding this comment.
When another connection revokes the reference or rotates the database UUID while authorize_scope is running, the earlier lookup contains the now-stale revoked_at and authority state, while this post-authorization query fetches only the target and revision. Resolution therefore returns the protected binding successfully even though revocation or clone rotation completed before disclosure; perform the reads in one consistent transaction or recheck authority and revocation in this final lookup.
Useful? React with 👍 / 👎.
| actual_columns = [ | ||
| str(item[2]) | ||
| for item in conn.execute(f"PRAGMA index_info({index_name})").fetchall() | ||
| ] if row is not None else [] |
There was a problem hiding this comment.
When a completed registry has an expected-name index recreated as UNIQUE with the same columns, this verification still reports an exact healthy schema because PRAGMA index_info does not expose uniqueness. For example, a unique scope index passes this check but makes the second reference issued for the same kind and scope fail with UNIQUE constraint failed, rather than failing closed during authority validation; also inspect the index-list uniqueness and partial flags.
Useful? React with 👍 / 👎.
| return json.dumps( | ||
| value, | ||
| ensure_ascii=False, | ||
| allow_nan=False, |
There was a problem hiding this comment.
When a binding contains an unpaired surrogate decoded from input such as "\ud800", ensure_ascii=False returns a Python JSON string containing that surrogate instead of rejecting it. Issuance passes the authorization boundary and then fails with an uncaught UnicodeEncodeError when SQLite encodes the value as UTF-8, rather than returning the promised typed invalid_request; reject non-scalar Unicode values during canonicalization or serialize them safely.
Useful? React with 👍 / 👎.
|
Closing in favour of #505, the maintenance roll-up — this is commit 2 of that stack. Nothing is dropped — the commits are carried across unchanged, so the review history here stays meaningful and the work is not rewritten. The consolidation is packaging: a system-level change reads better as one ordered stack than as several PRs that have to be merged in the right sequence to make sense. Apologies for the churn on your queue. |
1 participant
Summary
retrieval_references_v1server-side registry without incrementing numeric schema version 5;rh1_envelope codec plus issue, resolve, revoke, and explicit-clone-rotation primitives that store only SHA-256 token digests;Why
Raw database-local IDs and positional cursors are locators, not durable references. They cannot distinguish an ordinary restart from database replacement, detect target revision drift, or prove that the current trusted host context is authorized for the stored scope.
This PR implements only the additive V1 foundation needed by #476: durable logical-database identity, a digest-only opaque-reference registry, strict serialization and typed failures, and host authorization boundaries. It intentionally does not emit references from public tools, add pagination/adapters, define principal or tenant schemas, or add MAC/signing/key management.
The registry and database UUID are governed by named migration provenance while the existing numeric schema version remains 5. Completed healthy migrations are read-only; incomplete, damaged, or exact-shape-but-unmarked authority state is rejected rather than silently adopted or regenerated.
Validation
python -m pytest tests/test_retrieval_reference_contract.py tests/test_retrieval_reference_safety.py tests/test_db_bootstrap_fts.py tests/test_schema_stamp_remediation.py -q -o addopts=->93 passed, 4 xfailedpython -m pytest tests/test_lcm_core.py tests/test_lcm_engine.py tests/test_packaging_install.py tests/test_tool_contracts.py tests/test_lcm_command.py -q -o addopts=->1148 passed, 1 skippedpytest -q-> broad run remains red only on four failures independently reproduced on acceptedmain: two import-externalization assertions and two macOS/varversus/private/varpath-alias assertions; no candidate-path regression was foundscripts/validate_release.sh --full --keep-going --output /tmp/hermes-lcm-release-validation-retrieval-references-v1-> not rerun for this additive foundation; stacked prerequisite PR fix: serialize concurrent FTS bootstrap repair #486 completed the release validator, while this exact candidate completed the focused and neighboring matrices abovegit diff --check upstream/main...HEAD && git diff --check && git diff --cached --checkactionlint-> not applicable; no workflow files changedpython -m ruff check db_bootstrap.py retrieval_references.py tests/test_retrieval_reference_contract.pyand Python compilation -> passedNULLdigest rejection, and DDL-free healthy named-migration behavior passed; temporary source and bytecode were removedNotes
f0fd3608) because it extends the same bootstrap surface. Review the retrieval-reference commitbf44dc8independently; after fix: serialize concurrent FTS bootstrap repair #486 merges, the base range should collapse to this commit.tests/test_retrieval_reference_safety.pyremains outside this commit because it predates the implementation candidate.Refs #476