Repository navigation
sync: release/v1.5.2 — fork master forward to upstream v1.5.2 (policy-compliant source) - #4
Closed
tony-nexartis wants to merge 99 commits into
Closed
tony-nexartis wants to merge 99 commits into
tony-nexartis wants to merge 99 commits into
Conversation
…lupkede#135) The incremental-refresh and watcher paths constructed a fresh EmbeddingService per call; in serve the process-global LMDB registry refused the second open of the persistent cache, silently degrading every refresh to cache-less re-embedding. Thread the serve-wide pool through and reuse pool.get(model) so the process-shared cache is hit; CLI/standalone-MCP processes keep direct construction (no pool there).
- Chunk paths stored relative to the indexed project root so built DBs are portable across machines/containers (snapshot federation previously worked only because build path == serve path). One-time consequence: existing DBs refreshed by this binary see all files as changed and re-embed once. - index add --local: skip serve delegation, always build in-process. - index/index add: write a clean, unmangled build log to <project>/.codesearch.db/build.log when stdout is not a TTY (or --log-file). - current_git_head: strip GIT_DIR/GIT_WORK_TREE/GIT_INDEX_FILE/GIT_OBJECT_DIRECTORY from the child env so repo discovery keys off the given root (pre-existing test failure under push hooks).
…ocal, pool reuse)
…locks, status labels 2c13826 made chunk paths project-relative, which re-embedded every legacy index and duplicated chunks; it also exposed three latent failure modes. One commit carries four fixes for that fallout: - Key migration (index/key_migration.rs, cache/file_meta.rs): re-key legacy absolute FileMetaStore keys to project-relative form, rewrite the chunk paths in both stores, and sweep chunks no tracked file references — unchanged files are recognised as up to date instead of re-embedded, and the duplicates an earlier interrupted re-embed left are removed. - FTS writer (fts/tantivy_store.rs): prepare_commit joins and respawns the indexing workers and can abort half-way, leaving zero workers behind a live 10k-doc pipeline; that writer is now discarded and an error returned instead of silently dropping every later batch and blocking add_document. - Warmup (serve/mod.rs): register Warm and release the per-alias open_lock before the refresh so queries are answered from existing data; track the refresh with begin/end_indexing so the reaper leaves the repo alone and FSW does not start mid-warmup; queries acquire open_lock with a deadline instead of queueing behind a cold open forever. - Status (mcp/status.rs): report indexing/busy/held-in-process instead of mislabelling our own activity as locked-externally, and fetch live stats via try_read so status answers while a batch holds the store lock.
The orphan sweep ran on every refresh and could delete chunks a concurrent refresh had inserted but not yet recorded; it now runs only after a real migration, and the FSW no longer starts while warmup indexes. Path rewrites lock per batch so queries interleave, every re-keyed chunk is mirrored into FTS so a failed mirror is retried, and a killed FTS writer now returns an error instead of silently dropping documents.
BOIN-scale warmups run past MAX_INDEXING_SECS (30 min), so the lazy stale-marker eviction dropped the marker mid-refresh and the idle reaper could evict the repo while the refresh still held its stores (LMDB double-open / todo flupkede#131 shape). perform_incremental_refresh_with_stores now fires an IndexingHeartbeat at start, before each slow phase, and per batch; warmup passes a closure that renews its marker via ServeState::renew_indexing (upsert, so a raced eviction cannot strip protection for the rest of the run). Other callers pass None for now.
The FSW's completion callback cleared the whole alias entry with a plain bool, so a watcher refresh finishing mid-run erased a concurrent force reindex's marker — the reaper could then evict a repo the reindex still held (LMDB double-open shape). active_reindexes now maps alias -> owner -> last heartbeat (IndexingOwner: Warmup/Watcher/Reindex/Symbol); begin/end/ renew are owner-scoped, is_indexing evicts only stale owners and still cancels a leaked index task, and begin_indexing keeps rejecting while any owner's marker is fresh. Adds the race, exclusivity and per-owner eviction tests. Review-fixes: - [Critical] Empty-map cleanup dropped the shard guard before remove(alias), so a concurrent begin_indexing insert landing in the gap was erased (re-opened the plain-bool marker race) → all three sites (is_indexing, end_indexing, FSW callback) now use atomic remove_if(alias, |_, m| m.is_empty()) which holds the shard lock across predicate + removal.
index_status_impl's group fan-out still blocked on vector_store.read(): an indexing run holds the write lock for whole batches (build_index for minutes), so a group status call queued behind it. It now uses try_read like list_projects; a busy store counts as a failed stats read with a caller-facing warning naming the busy lock — never silently skipped.
… reader A live indexer holds vector_store's write lock for the whole batch; every read-side caller blocked on it forever, so one slow/stuck reindex hung every query. New CODESEARCH_STORE_LOCK_WAIT_SECS-configurable bounded_vector_read() (default 300s) wraps every direct vector_store.read().await in mcp/mod.rs (with_vector_store_read_for, with_vector_store_read_multi), mcp/runtime.rs (require_prebuilt_index_ready) and every per-store fan-out site in mcp/find.rs, mcp/get_chunk.rs, mcp/graph.rs, mcp/literal_search.rs, mcp/search.rs, mcp/explore.rs — each site now attributes a timeout the same way it already attributes a store error (note_store_failure / push_store_warning, per its existing warnings channel), so a busy store degrades one caller's answer instead of hanging it. FTS store reads are left unbounded — short-duration ops, out of scope.
The prior commit bounded vector_store reads but deliberately left fts_store.read().await unbounded. A live indexing batch holds both locks for its duration, so a stuck reindex could still hang every literal-search/find caller through the FTS side alone. New bounded_fts_read() (src/mcp/mod.rs, same CODESEARCH_STORE_LOCK_WAIT_SECS wait as bounded_vector_read) wraps the FTS lock at all four direct read sites: with_fts_store_read_for's two branches, the per-store loop in with_fts_store_read_multi (attributes the timeout into `failures`, same as its vector-store sibling), and runtime.rs's require_prebuilt_index_ready.
Final review of the 6-stage fallout fix found a narrow window: warmup registered RepoState::Warm before calling begin_indexing(Warmup), so a concurrent query landing in that gap could see is_indexing() == false and let FSW start a Watcher-owned refresh racing warmup's still-loading file_meta — the exact hazard the per-owner marker refactor exists to prevent. Fix: acquire the Warmup marker before the repo becomes visible in self.repos, so FSW's gate always sees it.
Pre-push leak scan flagged 'BOIN' (client codename) in three rationale comments about warmup duration at scale. Reworded to 'large repos' / 'large-repo' — no behavior change.
find(kind="usages") names its target via 'symbol', not file/path — edit-guard-post hooks now recognize it so both find_impact and find attribute to the same key.
…ath-migration Fix warmup-lock fallout after path migration
`Path::has_root()` only recognizes a Windows drive-letter path (`C:/...`) as rooted on Windows. On the Linux CI runner it saw such a path as relative, so `prefix_path_with_alias` wrongly alias-prefixed an out-of-root Windows path instead of leaving it unprefixed — failing mcp::tests::test_path_prefix_preserves_path_outside_root in CI. Added `is_portable_absolute()`, a platform-independent textual check (leading `/` or drive-letter + `:`), used instead of `has_root()`.
…rts ready Small models misread "Closed"/indexed:false as "index unavailable" and wrongly fall back to grep. Idle = index on disk, stores evicted by the idle reaper, auto-reopen on next query — i.e. searchable, first query slightly slower. Aggregated index_status now reports indexed:true + status:"ready" when any repo has an index on disk, with an explicit "Indexed and searchable: N" hint; "no_index" only when nothing is on disk. Wire string becomes "idle" (TUIs keep tolerating legacy "closed"). Review-fixes: - [Important] aggregation branches untested → extracted pure helper aggregate_serve_index_status (responses.rs), index_status_impl calls it, 3 unit tests pin all-idle→ready/indexed, nothing-on-disk→no_index, empty hub→no_repos - [minor, pre-existing] all-Readonly hub still reports no_index → deferred, out of this change's scope (requirement enumerated open+warm+idle); follow-up candidate
status_cell's string match now delegates to pure status_cell_spec -> (String, Style) so tests can assert text+style without a Cell style getter (ratatui 0.30 Cell::style is a setter). 3 tests: idle+closed gray cell, Idle label mapping, unknown-status fallback distinct from idle. Review-fixes: - [Important] status_cell/detail_status_style string arms (idle | closed) had no test — silent _ fallback could regress legacy-peer tolerance unnoticed → 3 tests in src/serve/tui_common.rs pin cell text/style and label mapping for idle, legacy closed, and the fallback arm
…status Direct push to develop bypassed bump-develop.yml (fires on PR merges only), so the patch bump is done by hand; CHANGELOG gains the pending [1.4.11] section (finalized with a date at tag time, per the file's convention note).
…de#192) - .cargo/config.toml: [target.x86_64-pc-windows-msvc] linker = "rust-lld" Bin-crate rebuild+link: 2m56s -> 1m32s (x2 faster dev cycle) - build.ps1: -Release now throws (debug-only policy); release artifacts are built by CI (release.yml) only - sccache: REJECTED — caches compilation, not linking (linking is the bottleneck), and disables incremental compilation; same verdict as audit flupkede#191 - tests/*.rs consolidation (53-binaries item in flupkede#191): N/A here — codesearch has 5 integration test binaries, consolidation not warranted - README.md "cargo build --release" (end-user build-from-source) kept: end users want optimized binaries; debug-only applies to the dev loop (AGENTS.develop.md already mandates it, CI builds debug)
…TS.develop.md; branch-local plan
…uilds build: debug-only policy + rust-lld linker (issue flupkede#192)
Bumps the cargo-minor-and-patch group with 4 updates: [thiserror](https://github.com/dtolnay/thiserror), [fastembed](https://github.com/Anush008/fastembed-rs), [rand](https://github.com/rust-random/rand) and [rmcp](https://github.com/modelcontextprotocol/rust-sdk). Updates `thiserror` from 2.0.20 to 2.0.21 - [Release notes](https://github.com/dtolnay/thiserror/releases) - [Commits](dtolnay/thiserror@2.0.20...2.0.21) Updates `fastembed` from 7.0.1 to 7.1.0 - [Release notes](https://github.com/Anush008/fastembed-rs/releases) - [Commits](anush008/fastembed-rs@v7.0.1...v7.1.0) Updates `rand` from 0.10.2 to 0.10.3 - [Release notes](https://github.com/rust-random/rand/releases) - [Changelog](https://github.com/rust-random/rand/blob/master/CHANGELOG.md) - [Commits](rust-random/rand@0.10.2...0.10.3) Updates `rmcp` from 3.4.0 to 3.4.1 - [Release notes](https://github.com/modelcontextprotocol/rust-sdk/releases) - [Changelog](https://github.com/modelcontextprotocol/rust-sdk/blob/main/release-plz.toml) - [Commits](modelcontextprotocol/rust-sdk@rmcp-v3.4.0...rmcp-v3.4.1) --- updated-dependencies: - dependency-name: thiserror dependency-version: 2.0.21 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: cargo-minor-and-patch - dependency-name: fastembed dependency-version: 7.1.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: cargo-minor-and-patch - dependency-name: rand dependency-version: 0.10.3 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: cargo-minor-and-patch - dependency-name: rmcp dependency-version: 3.4.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: cargo-minor-and-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…ipeline, find_impact self-heal, tests - trigger_symbol_rebuild generalized per language with a per-(alias,lang) in-flight claim (DashMap, stale takeover past indexing timeout); C#-only TUI status / last_scip bookkeeping stays gated on the language - evaluate_symbol_rebuild: generic gate (applies_to before availability); C# delegates to evaluate_csharp_rebuild; non-C# rebuilds only a MISSING index (drift stays watcher/branch driven — no-thrash) - run_phase_2_symbols (was run_phase_2_csharp_scip) iterates all installed languages; TypeScript now gets its initial build at startup - reindex_handler: maybe_rebuild_symbols gate-driven for every language, symbols=true still forces (previous unconditional behaviour) - add_repo_handler: maybe_rebuild_symbols after restart_fsw — closes the runtime-add gap (registered while serve runs never saw the startup pass) - find_impact: missing definitions index (has_index false) starts a fire-and-forget heal_missing_symbol_index (serve mode, alias re-resolved from config so remote mounts are never healed locally) and the empty NotFound answer carries a warning marking it UNKNOWN; a successful resolution keeps its payload to the persisted-warnings channel - tests: claim reject/stale-takeover, gate applicability-before-availability, heal no-op outside remit, force skips non-applicable, never-built index warns on the empty answer (fails without the fix), phase-2 composition wires candidate through to trigger, serve-mode self-heal runs the rebuild Review-fixes: - [Test] no coverage reached run_phase_2_symbols wiring or the serve-mode heal spawn → added run_phase_2_symbols_wires_candidate_through_to_trigger and find_impact_self_heal_runs_the_rebuild_in_serve_mode (dummy helper via env override; rebuild failure observable as CSharpIndexStatus::Error) Validation: cargo fmt --check clean, clippy --all-targets -D warnings clean, cargo test --lib --bins: 761 passed / 0 failed.
release: 1.5.1
…advisories are not index warnings
…t-noise fix(csharp): restore before workspace load; NuGet audit advisories are not index warnings
…rning-severity workspace diagnostics to INFO
…HOME tests global_codesearchignore_path_returns_home_codesearch_dir reads a global-path accessor that resolves via CODESEARCH_HOME but was not marked serial, so it could run concurrently with the serial tests that set/clear that env var and observe a relocated or rejected root. Marked serial like its siblings.
fix(csharp): restore only when assets are stale or missing; Warning-severity workspace diagnostics are INFO
…e background Killing the restore at the wait ceiling wasted all of its work and left assets missing, so every reindex re-paid the same wait and stayed degraded (a poison loop for exactly the biggest solutions). On timeout the restore now keeps running detached; this load proceeds degraded and the next reindex picks up the fresh assets. Redirected pipes are drained in the background so the child can neither block on a full pipe nor hold the serve process's stderr past EOF. Timeout warning moved to RestorePolicy.StillRunningWarning with a test pinning the detach contract.
fix(csharp): a timed-out restore is never killed - it continues in the background
Bumps the cargo-minor-and-patch group with 3 updates: [uuid](https://github.com/uuid-rs/uuid), [rmcp](https://github.com/modelcontextprotocol/rust-sdk) and [libc](https://github.com/rust-lang/libc). Updates `uuid` from 1.26.1 to 1.27.0 - [Release notes](https://github.com/uuid-rs/uuid/releases) - [Commits](uuid-rs/uuid@v1.26.1...v1.27.0) Updates `rmcp` from 3.4.1 to 3.5.0 - [Release notes](https://github.com/modelcontextprotocol/rust-sdk/releases) - [Changelog](https://github.com/modelcontextprotocol/rust-sdk/blob/main/release-plz.toml) - [Commits](modelcontextprotocol/rust-sdk@rmcp-v3.4.1...rmcp-v3.5.0) Updates `libc` from 0.2.189 to 0.2.190 - [Release notes](https://github.com/rust-lang/libc/releases) - [Changelog](https://github.com/rust-lang/libc/blob/0.2.190/CHANGELOG.md) - [Commits](rust-lang/libc@0.2.189...0.2.190) --- updated-dependencies: - dependency-name: uuid dependency-version: 1.27.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: cargo-minor-and-patch - dependency-name: rmcp dependency-version: 3.5.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: cargo-minor-and-patch - dependency-name: libc dependency-version: 0.2.190 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: cargo-minor-and-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…p/cargo-minor-and-patch-f34b24f6f2 chore(deps): bump the cargo-minor-and-patch group with 3 updates
…kill detach, dep refresh
release: 1.5.2
…ter) pull_request_target executes this workflow as it exists on the DEFAULT branch, and the default branch is master — the fix merged to develop (flupkede#332) only lands here at the next release sync, so every develop-PR bump meanwhile still skips the csproj and drifts the helper stamp from the crate version (deploy warning 'codesearch 1.5.2x vs scip-csharp 1.5.9/1.5.2x'). Same fix as flupkede#332: rewrite whatever <Version> value is there and hard-verify before the commit. Review: @Reviewer unavailable (usage limit); worker self-review — identical hunk as the reviewed flupkede#332 change, YAML parse OK.
…master fix: the per-merge bump never synced the helper version stamp (on master)
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 683be85997
ℹ️ 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".
…on (chunk ids are store-local, never globally unique) + csharp restore-policy P1/P2 — proven cargo check + cargo test 157 + dotnet test 7 Signed-off-by: Tony Rose <174962249+tony-nexartis@users.noreply.github.com>
Author
Ready for a non-author merge — release/v1.5.2 P1/P2 landed
|
Author
Disposition: BLOCKED — gated verb support landed; the D1 review path needs a coordination-plane wideningDeep-dive result (fork vs upstream). Upstream is Upstream branches not on What is now landed (PR #431, Why this PR still cannot land through the gated verb. The gated verb requires an approved shared-D1 change review (
Why this is routed, not rushed. Rebuilding Recommendation. Execute the coordination-plane base-enum widening as its own audited wave (local-D1 red-before/green-after proof → remote apply → console deploy), then land this PR via Mirror policy note: the fork is a supply-chain mirror ( |
… bad project= alias now returns a structured project_unresolved envelope with available_projects/groups instead of the bare "Unknown alias 'x'" string; + unit test (cargo test 812 pass, 1 pre-existing macOS /private symlink failure in db_discovery, unrelated) Signed-off-by: Tony Rose <174962249+tony-nexartis@users.noreply.github.com>
Author
Stacked on this PR: P1 hardening — classify the unknown-project refusalPushed
Proof: Baseline + roadmap for the rest of the tuning round (measured MCP schema tokens, index footprint, query-latency host-load caveat, external RRF/schema-tax SOTA): |
Author
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
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.
Fast-forward the Nexartis/codesearch fork master to upstream flupkede/codesearch v1.5.2 (97 commits), from a
release/*source branch soprotect-master(check-source-branch) passes by the repo's own policy.Supersedes #3 (which used a
sync/*source branch, refused byprotect-master.ymlby design).Two open code findings from the #3 bot review are still to disposition (they are upstream v1.5.2 code, inherited by the sync):
src/mcp/search.rs—prefix_paths_with_aliaskeys the fan-out map by local chunk id, so two repos with the same id collide and one alias overwrites the other → wrong repo labels on agroup=search. Fix: carry(alias, chunk_id).helpers/csharp/RestorePolicy.cs— restore staleness checks only the.csprojtimestamp; includeDirectory.Packages.props/Directory.Build.props/packages.lock.json/NuGet.config.Binary supply chain for v1.5.2 is already shipped (mirror release + install pin); this PR is source-mirror hygiene.