feat(git): git.digest paged ingest + URL clone cache (ADR-088 Amendment 1)#761
Merged
Conversation
…ch-clone cache (ADR-088 Amendment 1)
…iction safety, staged clone-cap enforcement, max_items parsing ADR-088 Amendment 1 fix round (codex APPROVE-WITH-FIXES): - HIGH: gh pr/issue list paged past the hard-coded --limit 1000 window. Replaced the single-fetch with an explicit --search "sort:updated-asc updated:>=<floor>" paging loop (ingest.rs) that advances the floor between calls and only reports done:true when a page proves the remote window is exhausted (fewer than PAGE_LIMIT items returned) -- a full page with the local budget not yet exhausted no longer silently reports done:true. The decision logic (decide_page_outcome/PageOutcome) is pulled out pure and unit-tested independent of gh/network/DB. - HIGH: cache eviction (cache.rs::evict_lru) previously treated every direct child of the scratch root as an owned clone. Added is_owned_entry (16-hex cache-key-shaped name + .git dir + .khive-last-used marker) and gated eviction on it, so a KHIVE_GIT_DIGEST_SCRATCH_ROOT override pointed at a broader directory can never lose unrelated operator data. - MEDIUM: per-clone size cap was enforced after the clone/fetch had already written past it. ensure_clone now clones into a private staging directory outside the cache root, measures it there, and only renames it into the addressable cache slot once under the cap -- an oversized clone never enters the cache slot or evict_lru's accounting. Doc comment states precisely what is (and isn't) guaranteed: the cap gates cache-slot entry, not the staging clone's transient disk usage. - MEDIUM: max_items parsed via Value::as_u64, so a negative value silently fell through to the 500 default instead of clamping to the 1 floor. handlers.rs now parses as i64, rejects non-integer values outright, then clamps to 1..=2000. - LOW: regenerated docs/guide/api-reference.md's verb count (75->76) and added the git pack's verb table entry for git.digest. Gates (crates/, CARGO_TARGET_DIR=worktree-local target): fmt --check rc=0, clippy -D warnings (khive-pack-git, kkernel) rc=0, cargo test -p khive-pack-git rc=0 (54 tests: 35 unit incl. 12 new + 19 integration incl. 4 new), cargo test -p kkernel rc=0, cargo check --workspace rc=0.
…s, clean up staging dir on clone failure ADR-088 Amendment 1 fix round r2 (codex APPROVE-WITH-FIXES, 2 residuals from the r1 fix itself; r1's five findings all confirmed closed): - HIGH: the r1 paging rewrite (fetch_pr_page/fetch_issue_page, ingest.rs) dropped the --state all flag the single-fetch version had, so gh's open-only default silently made closed issues and closed/merged PRs vanish from ingests -- exactly the historical provenance the pack exists to capture. Restored --state all on both gh pr list and gh issue list calls. Added a fake-gh argv assertion (gh_boundary_contract_and_partial_ ingest_failure in tests/acceptance.rs) that every logged pr/issue list invocation contains "--state all", so a future regression here fails a test without needing live GitHub access. - MEDIUM: ensure_clone's staging-then-rename path (cache.rs) cleaned up the staging dir on dir_size/over-cap/rename errors but not on the clone() call itself failing -- git can create and partially populate the destination before erroring (bad ref, auth failure, network drop), and evict_lru deliberately never touches non-owned .staging-* names, so repeated clone failures could accumulate unbounded scratch data. Wrapped the clone call in .inspect_err to remove_dir_all the staging dir on failure. Added ensure_clone_cleans_up_staging_dir_on_clone_failure, using a nonexistent local path as the clone source (fails fast, no network needed) and asserting no .staging-* directory remains under the scratch root afterward. Gates (crates/, CARGO_TARGET_DIR=worktree-local target): fmt --check rc=0, clippy -D warnings (khive-pack-git, kkernel) rc=0, cargo test -p khive-pack-git rc=0 (55 tests: 36 unit incl. 1 new + 19 integration, one extended with the --state all assertion), cargo test -p kkernel rc=0, cargo check --workspace rc=0.
ohdearquant
approved these changes
Jul 9, 2026
ohdearquant
left a comment
ohdearquant
left a comment
Owner
There was a problem hiding this comment.
Approving review per merge flow v2: ADR-088 Amendment 1 contract verified at the diff (clamped max_items + done/cursor resumability with the frozen-cursor tie invariant, https-only source resolution with explicit ssh/git/scp rejection, staging-dir clone with failure-path cleanup and measure-before-move size cap, hook/prompt hardening, fail-open reference extraction with precedes parent edges). Codex r3 clean APPROVE on record.
The smoke-test surface tripwire (tests/smoke_test.py) and the verb-count lines in CLAUDE.md, AGENTS.md, and docs/guide/api-reference.md all said 76 with 'git contributes 0 verbs'. git.digest makes it 77; the api-reference pack table also understated brain at 14 (brain.event_counts made it 15). This is exactly the drift the tripwire exists to catch — CI caught it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ohdearquant
pushed a commit
that referenced
this pull request
Jul 9, 2026
…g (S1) (#762) * feat(runtime): resolve_reference capability + recently-referenced ring (S1) Implements Slice 1 of the unified-verb draft ADR (.khive/workspaces/20260709/unified-verb/DRAFT-ADR-unified-verb.md): - khive-runtime: reference_ring module — a bounded, per-(namespace, actor) daemon-warm ring (default N=64, TTL=30min, size-or-age eviction). Never persisted, no schema change. Wired into VerbRegistry as an Arc-shared field; admission happens at the dispatch boundary in dispatch_with_identity, gated by ring_admissions_for() which admits ONLY singleton create/get/update/delete/merge/link results (both link endpoints) — search/list and bulk shapes never admit. - khive-runtime: reference_resolution module — resolve_reference(), the Layer-0 deterministic resolver. Three ordered stages, never a silent pick among close candidates: (1) id-string passthrough (UUID / 8+ hex prefix) through the existing resolve_by_id/resolve_prefix_unfiltered path; (2) ring exact/substring match; (3) hybrid_search fallback with a decisive-margin auto-resolve rule. Returns Resolved/Ambiguous/NotFound. - khive-pack-kg: new read-only resolve(refs, kind?, limit?) verb (18th kg handler), thin wrapper over resolve_reference. No mutation, no plan execution — that is S2's ask verb, out of scope here. - Bumped the three verb-count pins (kg_pack_exposes_18_handlers, pack_verbs_returns_eighteen, smoke_test.py total==77) that the new verb necessarily moves. Tests: 17 new (11 reference_ring, 6 reference_resolution unit tests in khive-runtime; 7 dispatch-level integration tests in khive-pack-kg covering ring admission, actor isolation, ambiguity, and the search/list non-admission gate condition). Full khive-runtime + khive-pack-kg suites green (848 tests), fmt/clippy/workspace-check clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(runtime): address codex review findings on resolve_reference/ring (S1) Fixes all six findings from the b1d2a3b4 review (review_resolve_s1.md), 2 High / 4 Medium: 1. [High] Ring admission (pack.rs) keyed on the gate-resolved `ns`, which can be a non-local `default_namespace`; ring lookup (reference_resolution.rs) keyed on `token.namespace()`, always `local` on the default dispatch path (ADR-007 Rule 0/3b). Admission now keys on `token.namespace()` too, matching lookup. Regression: resolve_via_ring_survives_non_local_default_namespace. 2. [High] Ring admitted any create/get/update/delete result carrying an `id`, including notes, and fell back to a note's `content` snippet as the display name. Added `substrate_admits_as_entity` (checks the `entity_type`/`content` key shape khive-pack-kg's handlers already return, plus edge/event `kind` tags and delete's echoed `kind` param) and dropped the content-fallback in `display_name`. merge/link admission unchanged (merge is entity-only by construction; link was already correct per review). 3. [Medium] The outer `HashMap<(namespace, actor), VecDeque<_>>` never dropped empty/stale keys or bounded key count. Added `prune_outer_map`: empty/stale keys drop on every admit/snapshot, and a new `DEFAULT_MAX_OUTER_KEYS` (4096) budget evicts least-recently-touched keys once exceeded (the just-admitted key is exempt from that pass). 4. [Medium] `lock().expect(...)` in `admit`/`snapshot` could panic a successful dispatch on mutex poisoning. Added `lock_state()`, which recovers via `into_inner()` with a `tracing::warn!` instead of panicking. 5. [Medium] Singleton hybrid-search resolution compared raw RRF scores (~1/61 typical) against a 0.7 absolute confidence bar, making the fallback functionally unable to resolve. Replaced with a presence rule: a lone candidate (nothing to be ambiguous against) or a decisive-margin winner resolves at a new, documented `SEARCH_RESOLVED_CONFIDENCE` constant, distinct from the ring's confidence bands; raw RRF stays exposed only via `ReferenceCandidate.score` in `Ambiguous` listings. 6. [Medium] Full-UUID passthrough used `resolve_by_id` (entity+note scope) while prefix passthrough scanned entity+note+event+edge — an edge's full UUID returned NotFound while its prefix resolved. Both branches now resolve entity ids only, identically, documented inline. Regression: resolve_id_string_passthrough_is_entity_only. Also rebases onto origin/main (PR #761, git.digest) — smoke_test.py's pinned verb count moves 76->78 (context #588 + resolve, the 17th/18th kg-substrate bare verbs; git contributes git.digest). Gates (worktree-local CARGO_TARGET_DIR, run from crates/): fmt --check, clippy -p khive-runtime -p khive-pack-kg --all-targets -- -D warnings, cargo test -p khive-runtime -p khive-pack-kg (789 passed), cargo check --workspace. All green. tests/smoke_test.py also verified directly against a built binary (total==78). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(runtime): prune the whole outer ring map from snapshot too (r2) Codex r2 (review_resolve_s1_r2.md), one remaining Medium: prune_outer_map had a single call site (admit), so a read-only daemon that only ever calls snapshot() never pruned any OTHER actor's stale outer-map key — only the queried key got cleaned up (and only via the ad-hoc if-empty-remove this replaces). snapshot() now calls the same prune_outer_map sweep admit() does, over the whole map, after evicting the queried key's own stale entries. Regression: snapshot_prunes_other_stale_keys_it_did_not_query (two actors age out; only one is snapshotted; asserts the other's stale key is gone too). Gates (worktree-local CARGO_TARGET_DIR, run from crates/): fmt --check, clippy -p khive-runtime -p khive-pack-kg --all-targets -- -D warnings, cargo test -p khive-runtime -p khive-pack-kg (1156 passed across all binaries), cargo check --workspace. All green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(kkernel): update kg verb-count pins for resolve (17 -> 18) The resolve verb (S1) raises the kg pack surface to 18 verbs; update the pack_introspect regression, the pack-kg crate docstring, and the contract fixture manifest that all pinned 17. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(kkernel): add resolve to the kg bare-verb allowlist (contract test) The verb_namespace_contract test pins the bare-name allowlist separately from the pack_introspect count; add resolve and drop the count from the allowlist's name so it cannot rot again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(kkernel): drop remaining hardcoded verb counts from contract-test comments Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: remove internal review artifacts committed by mistake Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
2 participants
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.
Implements the ADR-088 Amendment 1 lane:
git.digestpaged GitHub ingest and URL clone caching.ghfetches via--search "sort:updated-asc updated:>=<floor>"with a puredecide_page_outcome/PageOutcomedecision function (7 unit tests);done:trueonly when a page proves the window exhausted; floor-stall returnsdone:falseinstead of spinning.--state allon both pr/issue paged fetches, regression-locked by a fake-ghargv assertion (every logged invocation must carry it)..staging-<uuid>dir with pre-cache size check, same-filesystem rename into the cache slot; staging cleaned on clone failure, over-cap, size-probe error, and rename error paths.evict_lrugated byis_owned_entry(16-hex name +.git+.khive-last-usedmarker) so foreign dirs are never removed.max_itemsparsed asi64, non-integers rejected, clamped1..=2000(4 boundary tests).precedescommit→commit allowed via pack-levelEdgeEndpointRule(GIT_EDGE_RULES).Gates: fmt, clippy
-D warnings,cargo test -p khive-pack-git(55),cargo test -p kkernel,cargo check --workspaceall green. Three codex review rounds (r1 APPROVE-WITH-FIXES 2H/2M/1L, r2 APPROVE-WITH-FIXES 1H/1M on the r1 fixes, r3 fix commit a225b81) — narrow r3 re-review in flight.🤖 Generated with Claude Code