Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 9 additions & 20 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,38 +14,27 @@ more PRs land; when the release is actually tagged, the same section is
finalized in place with a date — no renaming/migration step needed.
-->

## [1.2.15] (unreleased)

### Fixed

- **`index rm` against a running serve: the Layer-2 acceptance path is now pinned end-to-end (todo #48).** The server-side unload flow (FSW stop → await shutdown → unregister → lock-class retry delete) and the CLI's serve delegation existed, but nothing exercised the *composition*: CLI `remove_from_index` → health probe → `DELETE /repos/:alias` → serve deletes the DB directory **without being stopped** → entry gone from repos.json → later queries a clean "Unknown alias" (no zombie stores). A new serial, hermetic integration test in `src/serve/tests.rs` drives exactly that: it spawns a real axum router with the genuine `remove_repo_handler` and `health_handler` over a `ServeState` seeded from a temp `repos.json`, points `CODESEARCH_REPOS_CONFIG`/`CODESEARCH_SERVE_PORT`/`CODESEARCH_SERVE_HOST` at it (`EnvRestore`), runs the actual CLI code path, and asserts the DB dir is deleted, the registration is gone, and a second removal fails with "Unknown alias". Mutation-verified: disabling the delegation makes the test fail on the zombie-store assertion. Getting the port right exposed a subtle trap now documented in the test: `TcpListener::local_addr()` returns a `SocketAddr` — binding the *address* instead of `.port()` silently produces `127.0.0.1:127.0.0.1:<port>` URLs and a port-parse fallback to the default 39725, which pointed the delegation at the developer's REAL serve (the test's isolation guard is exactly what caught it).

## [1.2.14] (unreleased)

### Fixed

- **A scale-to-zero cold start on a federated peer surfaced as a raw, undiagnosable error instead of a retry (todo #58).** `get_chunk`/`search` against a remote peer that answered 503 (non-JSON body — an Azure Container App waking from idle) failed immediately with `remote /chunk returned non-JSON body (http=503 Service Unavailable)`, giving the caller no way to tell a transient cold start apart from a broken mirror. Observed consequence: an agent abandoned a verification that was one retry away from succeeding, and read "503" as "mirror down" — pushing toward fallbacks that cannot work. Both federation read paths now retry transient statuses (502/503/504) a bounded number of times (3 attempts, 3s/8s backoff — [`REMOTE_PEER_RETRY_ATTEMPTS`] in `src/constants.rs`, test-overridable via `CODESEARCH_REMOTE_RETRY_BACKOFF_MS`) inside the already-active tool call, which is not a poll: the "never contact a federated peer on a cadence" design constraint is untouched. Most cold starts never reach the caller. If the peer is still transient-failing after the retries, the message names the likely cause and the remedy (`remote /chunk did not respond in time (http=503 after 2 retries) — likely a cold start on a scale-to-zero host; retry the same call in ~30s`) so "temporarily unavailable" stays distinguishable from "misconfigured/auth failed". Non-transient statuses (4xx, the peer's own 5xx tool errors) are NOT retried — a real answer fails identically on retry, just slower. Transport errors are not retried either (the per-request timeout already spent the budget). Four tests pin it: 503→200 get_chunk succeeds after exactly 3 attempts, persistent 503 carries the cold-start hint + retry count and stops at the configured attempts, a 500 is answered after exactly 1 request (no retry), and search gets the same treatment (it shared the identical pre-fix code shape).

## [1.2.13] (unreleased)
## [1.3.0] - 2026-08-15

### Added

- **`GET /indexing?path=<absolute path>` — per-repo freshness probe, and grep-guard now waits instead of forcing a grep fallback after a branch switch (todos #54/#55).** Two related fixes shipped together. (1) `codesearch serve` exposes a cheap new endpoint that resolves an absolute filesystem path to its containing registered repo (longest-root-wins, component-boundary match so `/x/alpha` never matches `/x/alpha-x`) and reports `{"covered":bool,"alias":..,"indexing":bool}` — `indexing` is true while that repo has an active (non-stale, lazily-evicted) reindex in flight, which includes the full refresh the file watcher fires on every branch switch. Same auth class as `/status` (open on localhost, bearer-protected on network binds); `/healthz` deliberately stays the only always-unauthenticated endpoint — liveness and freshness are different questions. (2) The Claude Code `grep-guard` hook (both `.ps1` and `.sh`, kept in sync) now probes this endpoint when the serve hub is live: if the target repo is mid-reindex, the deny message becomes a **wait-and-retry instruction** (sleep 15-30s, then re-run the codesearch call) instead of the standard "use codesearch" one — searching a mid-rebuild index returns stale/empty results, which previously pushed the agent into a manual `(approved fallback)` grep on every routine checkout. The probe is skipped silently (standard deny) on serves that predate the endpoint, so hook and server versions mix freely. Additionally fixed in the same hooks: repo resolution now follows the **grep target** (the git root of the path being searched) instead of the hook's cwd — an absolute-path Grep into a different indexed repo previously looked "external" against the cwd's repo root and slipped the guard uncovered.

## [1.2.11] (unreleased)
- **CI now checks that every PR into `develop` touches `CHANGELOG.md`.** Added after several PRs (#193, #196/#197) landed with no changelog entry and nobody could later tell which bugs a given release actually fixed. The check (`.github/workflows/changelog-check.yml`) is visible-not-blocking — the same `--admin` merge override that bypasses this repo's review ruleset also bypasses a required status check, so making it required would add ceremony without enforcement; instead a missing entry is a red X on the PR and in `gh pr checks`, and skipping it deliberately requires labeling the PR `no-changelog` (for genuinely user-invisible CI/tooling churn). The diff is taken from the merge base so a rebase or develop-merge into the branch cannot false-pass the check with develop's own changelog commits. Also in this PR: env-mutating tests are now `#[serial]` with panic-safe restore (`crate::testing::EnvRestore`), and the `index rm` regression tests pin `CODESEARCH_SERVE_PORT` to an in-test reset-server so their serve-delegation probe can never fire a live `DELETE` at a developer's running serve.

### Fixed
- **Literal-mode search results fabricated a `chunk_id: 0` that could silently resolve to the wrong file (todo #51).** `search(mode="literal")` hits carry no chunk id (literal search pinpoints a line, not a chunk), but both places that flatten literal results into the merged/federated response shape rendered the absent id as a real-looking `chunk_id: 0` (`.unwrap_or(0)`). A caller combining that fabricated 0 with the result source into a `get_chunk("<peer>/<alias>:0")` call got the wrong file back — no error, no ambiguity warning (reproduced against the federated `cloud/custom-kb` project). `SearchResultItem.chunk_id` is now `Option<u32>` and the field is omitted entirely for literal hits, so an id the server never returned cannot be constructed. Both fixed sites carry a red-verified regression pin. Also adds a store-level unit repro of the secondary cross-generation id-drift hypothesis (autoincrement ids are reused after a top-of-range delete on reopen), confirming the mechanism locally while the production two-cold-start comparison remains open — see AGENTS.md Open TODOs.

- **Chunk ids were reused across reopens after top-of-range deletes, making `get_chunk` silently return the wrong file (todo #51).** `VectorStore` derived `next_id` from the highest *live* key on every open, so deleting the chunks holding the highest ids (a routine event on the custom-kb replica: every rename is a delete+add) lowered the ceiling and the next open handed those ids to unrelated new content — a stale `get_chunk(id)` then resolved to a different file with no error. This is the mechanism the unit repro on `fix/custom-kb-chunk-id-drift` pinned. Fixed with a persistent high-water mark: the highest id ever assigned is stored in a new `meta` LMDB database (`id_hwm`), written in the same transaction as the chunks it covers, and `next_id = max(live max_key + 1, mark + 1)` on every open. Deleted ids stay dead forever (`get_chunk` → `Ok(None)`, a safe miss). A deliberate `clear()`/full rebuild wipes the mark with the data, so a new generation may restart at 0 — stale references then miss safely instead of aliasing new content. Legacy stores without the mark open unchanged (live-keys derivation, identical to previous behaviour) until their first insert persists the mark; snapshot/restore carries it automatically since it lives in the DB itself. Note: the repro tests on `fix/custom-kb-chunk-id-drift` assert the OLD reassignment behaviour and must be updated to the new safe behaviour when that branch merges with this fix. Four new tests pin the behaviour: top-of-range delete, full delete (counter never restarts at 0), `clear()` resets the generation, and legacy no-mark fallback.
Literal-mode

- **A broken vector store silently shrank or emptied search/find results instead of reporting itself (todo #57 review).** Six resolution sites in `src/mcp/mod.rs` flattened `get_chunk`'s `Err` into "chunk not found": the single-store literal-search resolver (`.ok()??` in a `filter_map`), the hybrid semantic+literal fusion path (closure mapped `Err` → `Ok(None)`, then the caller did `.ok()` on top), and both the multi-store and single-store branches of `find` definitions and `find` usages (`if let Ok(Some(chunk)) = store.get_chunk(..)` with no `Err` arm — the comment under one loop said "just skip it"). A dead store therefore rendered as an ordinary empty or short result with zero signal to the caller — the exact "search errors must not become empty results" defect class this repo has fought across nine review rounds; sibling handlers had the fix, these sites did not. All six now follow the compliant in-file pattern: multi-store loops bind the result and record the failure via `note_store_failure` into the handler's existing warnings channel; single-store closures propagate the `Err` with `?` so it reaches the handler's error exit; the hybrid fusion path notes the failure into `single_warnings` and stops hammering the dead store. Pinned by a new source-scanning integration test (`tests/store_err_swallow_detector.rs`, same approach as `caller_facing_literals.rs`) that fails the build on a direct `get_chunk(..).ok()` or an `if let Ok(Some(..)) = store.get_chunk(..)` scrutinee anywhere under `src/mcp/` — reintroducing either form on any site was confirmed to fail the test before it was merged.
Chunk ids

- **`codesearch index rm` could leave a repo unregistered while its (still-locked) database sat on disk untouched (todo #48).** The removal order was: unregister from `repos.json` and save it, *then* delete the `.codesearch.db` directory. When the delete failed — typically because a running `serve` instance (one the delegation probe didn't see: a second instance on another port, a stray CLI process, or `serve` having crashed without releasing its LMDB env) still held the files locked — the command errored out with the config entry already gone. The registry now claimed the repo didn't exist while its still-locked database remained on disk, with no way to clean it up except manually stopping the locking process; running the same `rm` command again did nothing, because `repos.json` no longer had an entry to remove. Fixed by reversing the order: the database directory is deleted first, and `repos.json` is only mutated once that succeeds. A failed delete now leaves the config untouched and prints an explicit "repos.json was NOT modified" message, so re-running the identical command after clearing the lock finishes the job. Also folded a pre-existing double-unregister in the "both local and global index exist" path into the same single call. Four regression tests cover the failing-delete, successful-delete, `--keep-config`, and global-only-entry cases.
A broken vector

### Added
`codesearch index rm` could

- **CI now checks that every PR into `develop` touches `CHANGELOG.md`.** Added after several PRs (#193, #196/#197) landed with no changelog entry and nobody could later tell which bugs a given release actually fixed. The check (`.github/workflows/changelog-check.yml`) is visible-not-blocking — the same `--admin` merge override that bypasses this repo's review ruleset also bypasses a required status check, so making it required would add ceremony without enforcement; instead a missing entry is a red X on the PR and in `gh pr checks`, and skipping it deliberately requires labeling the PR `no-changelog` (for genuinely user-invisible CI/tooling churn). The diff is taken from the merge base so a rebase or develop-merge into the branch cannot false-pass the check with develop's own changelog commits. Also in this PR: env-mutating tests are now `#[serial]` with panic-safe restore (`crate::testing::EnvRestore`), and the `index rm` regression tests pin `CODESEARCH_SERVE_PORT` to an in-test reset-server so their serve-delegation probe can never fire a live `DELETE` at a developer's running serve.
- **A scale-to-zero cold start on a federated peer surfaced as a raw, undiagnosable error instead of a retry (todo #58).** `get_chunk`/`search` against a remote peer that answered 503 (non-JSON body — an Azure Container App waking from idle) failed immediately with `remote /chunk returned non-JSON body (http=503 Service Unavailable)`, giving the caller no way to tell a transient cold start apart from a broken mirror. Observed consequence: an agent abandoned a verification that was one retry away from succeeding, and read "503" as "mirror down" — pushing toward fallbacks that cannot work. Both federation read paths now retry transient statuses (502/503/504) a bounded number of times (3 attempts, 3s/8s backoff — [`REMOTE_PEER_RETRY_ATTEMPTS`] in `src/constants.rs`, test-overridable via `CODESEARCH_REMOTE_RETRY_BACKOFF_MS`) inside the already-active tool call, which is not a poll: the "never contact a federated peer on a cadence" design constraint is untouched. Most cold starts never reach the caller. If the peer is still transient-failing after the retries, the message names the likely cause and the remedy (`remote /chunk did not respond in time (http=503 after 2 retries) — likely a cold start on a scale-to-zero host; retry the same call in ~30s`) so "temporarily unavailable" stays distinguishable from "misconfigured/auth failed". Non-transient statuses (4xx, the peer's own 5xx tool errors) are NOT retried — a real answer fails identically on retry, just slower. Transport errors are not retried either (the per-request timeout already spent the budget). Four tests pin it: 503→200 get_chunk succeeds after exactly 3 attempts, persistent 503 carries the cold-start hint + retry count and stops at the configured attempts, a 500 is answered after exactly 1 request (no retry), and search gets the same treatment (it shared the identical pre-fix code shape).

- **`index rm` against a running serve: the Layer-2 acceptance path is now pinned end-to-end (todo #48).** The server-side unload flow (FSW stop → await shutdown → unregister → lock-class retry delete) and the CLI's serve delegation existed, but nothing exercised the *composition*: CLI `remove_from_index` → health probe → `DELETE /repos/:alias` → serve deletes the DB directory **without being stopped** → entry gone from repos.json → later queries a clean "Unknown alias" (no zombie stores). A new serial, hermetic integration test in `src/serve/tests.rs` drives exactly that: it spawns a real axum router with the genuine `remove_repo_handler` and `health_handler` over a `ServeState` seeded from a temp `repos.json`, points `CODESEARCH_REPOS_CONFIG`/`CODESEARCH_SERVE_PORT`/`CODESEARCH_SERVE_HOST` at it (`EnvRestore`), runs the actual CLI code path, and asserts the DB dir is deleted, the registration is gone, and a second removal fails with "Unknown alias". Mutation-verified: disabling the delegation makes the test fail on the zombie-store assertion. Getting the port right exposed a subtle trap now documented in the test: `TcpListener::local_addr()` returns a `SocketAddr` — binding the *address* instead of `.port()` silently produces `127.0.0.1:127.0.0.1:<port>` URLs and a port-parse fallback to the default 39725, which pointed the delegation at the developer's REAL serve (the test's isolation guard is exactly what caught it).

## [1.2.10] - 2026-08-12

Expand Down
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "codesearch"
version = "1.2.17"
version = "1.3.0"
edition = "2021"
authors = ["codesearch contributors"]
license = "Apache-2.0"
Expand Down
Loading