Repository navigation
feat(index): versioned Bleve mapping with automatic migration and a real RebuildIndex - #1386
Merged
Merged
Conversation
Every <dialog class="modal"> now opens via showModal()/close() through a shared useDialogOpen composable instead of the `open` attribute or a `modal-open` class toggle, so a modal can never be painted over by the sidebar or header regardless of z-index (the Add Server modal bug, H4). Adds frontend/src/assets/z-index.css as the single source for the sidebar < header < dropdown < modal < toast scale and retires the ad-hoc z-40 on the sidebar's drawer-side. Also folds in two small header changes that touch the same files: Add-to-MCPProxy button state and header layout groundwork.
Cross-surface UX fixes with no dependency on later PRs in the spec:
- `/` lands on the Overview panel (interim step before Home in a later
PR); `/usage` and `/overview` stay deep-linkable (FR-051).
- Server detail and Settings tabs read `?tab=` on mount and write it
back with router.replace on change, keeping other query params
(FR-016).
- Settings is named "Settings" everywhere (sidebar, route title, H1,
document title), its tabs use line icons instead of emoji, and the
"Server Edition" tab is gated on the runtime edition
(systemStore.status.edition, now carried on every SSE status frame)
rather than on whether the config happens to have a server_edition
key (FR-056).
- Tool review states use one vocabulary — Approved / New, needs review
/ Changed, needs review — with the `awaiting` filter value removed;
the Tools page's "Needs review" stat is a link to /review, backed by
interim redirects to /servers (?status=needs_review) and
/servers/:name (?tab=tools) until a later PR ships the real review
views (FR-027, T026a).
- One pure function, contracts.AnnotationTier, computes a tool's tier
(read/write/destructive/unannotated) from its annotations; GET /tools
and GET /servers/{id}/tools carry it, and the Web Tools page and CLI
`tools list --tier` (`--risk` kept as an alias) use it instead of each
deriving their own — the CLI's old `--risk` read a field
(annotations.operation_type) that doesn't exist on a real payload, so
it matched nothing (FR-028, X11).
- Count charts (calls per tool, activity over time) use integer ticks,
and the excluded "never completed a call" group is titled "Calls to
unknown tools" (FR-074).
- "Add to MCPProxy" (Repositories + CLI `registry add`), flipping to
"Added ✓ · Open" after a successful add (FR-063).
- The header ProfileSwitcher is hidden until at least one profile
exists (FR-057).
…PR a) - ToolLabels.swift: one shared approval-state and tier label table (Approved / New, needs review / Changed, needs review; Read / Write / Destructive / Unannotated / Unknown), matching the Web/CLI vocabulary. ServerDetailView's approval badge and ToolsView's new tier badge both go through it. - ServerTool/SearchTool decode the backend-computed `tier` field (contracts.AnnotationTier) rather than deriving one from raw annotations. - ServerBrowseView: "Add to MCPProxy", flipping to "Added ✓ · Open" (opens the server) after a successful add.
make swagger, after adding contracts.Tier / Tool.Tier.
Verified findings against 109-a-quick-wins (PR-a of Spec 109).
FIXED:
- z-index: --z-header/--z-sidebar shipped inverted, so the sticky
header painted over the open mobile drawer sidebar instead of the
other way around (the same class of stacking bug this scale exists
to prevent, on the header/sidebar pair instead of sidebar/modal).
Swapped the token values, updated the doc comment and
z-index-scale.spec.ts's ordering assertion.
- useDialogOpen: never listened for the native `close` event a
showModal()-opened <dialog> fires on Escape, so the driving Vue
state desynced from the DOM the first time a user hit Escape on
Repositories.vue's three dialogs, UserTokens/UserServers/
UserActivity's dialogs, OnboardingWizard or ConnectModal — the
dialog stayed closed forever after that (no reopen). Added an
`onClose` hook plus a `close` listener guarded against looping back
on our own close() calls, wired every affected consumer's own
close/reset function through it. AddServerModal/AddSecretModal
(already safe via useModalA11y's capture-phase Escape handler) get
the same wiring for defense in depth. New test:
use-dialog-open-native-close.spec.ts.
- Search tier: GET /index/search always reported tier=unannotated
regardless of a tool's real annotations, disagreeing with the same
tool's tier on GET /servers/{id}/tools. The reported cause (a missing
"annotations" key in server.searchResultsToMaps's tool map) was one
level above the real one: the Bleve ToolDocument never stored
annotations at all, so a search hit could not carry them no matter
what the map-shaping code did. Added a stored (not indexed)
annotations_json field to the Bleve schema, round-tripped through
toolDocument/readToolMetadata, requested it in the shared search
Fields list, and now surface it in searchResultsToMaps. New tests:
bleve_annotations_test.go, search_tools_tier_test.go.
- Tools.vue: selectStatCard's toggle-to-total branch dropped the
filterApproval reset that existed before this PR removed the old
"pending" stat card. Total is the row's other "reset" gesture
(clearFilters resets both filters too); restored it. New test:
tools-total-card-resets-approval.spec.ts.
- macOS ServerBrowseView: the "Added checkmark Open" flow keyed
addedServers on the registry catalog's display name, but the backend
can assign a different name (empty name falls back to the entry id,
a conflict gets de-duplicated) and echoes it in the response body,
which AddServerResult discarded entirely. Now decoded and used,
matching the Web UI's `result.server?.name || server.name`. New
test: APIClientAddServerFromRegistryTests.swift.
- macOS ToolsView: the tier badge rendered nothing at all for a nil
tier (an older core not yet sending the field), while the Web UI
always renders one, defaulting to "Unannotated". Added
ToolRow.displayTier and used it unconditionally. New tests in
ToolLabelsTests.swift.
- cmd/mcpproxy/registry_cmd.go: registryAddErrorOutput's doc comment
was glued to registryAddMessage by a missing blank line, hiding it
from godoc. Reordered; cosmetic only.
REJECTED (false positive for this PR):
- scripts/test-api-e2e.sh's blanket `pkill -f "mcpproxy.*serve"` /
`pkill -f "launcher-server.*--port 39933"` in its cleanup trap is a
real, pre-existing operational risk (can kill another worktree's or
the user's own running instance), but git blame shows these lines
predate every commit in this PR and none of this PR's three commits
touch the file. No tasks.md for Spec 108/109 exists in this repo to
substantiate the finding's claim that removing them was a task this
PR was itself scoped to do. Left alone here rather than fixed blind
without the actual spec to guide the exact requirement (own-PID
tracking vs. a decoy-process proof) on a script every worktree's E2E
run depends on; flagged instead as a follow-up task.
DEFERRED (genuine, not implemented — infra, not a rejection):
- e2e/web-ui-sweep/visual-a11y-sweep.spec.ts has no elementFromPoint
assertion proving the z-index fix above holds in a real browser.
Adding one needs the real launcher (scripts/run-web-smoke.sh) and a
live Playwright run to confirm it actually catches the regression
rather than being vacuously true — this fix-and-verify pass could
not do that locally, so an unverified test was not committed;
flagged as a follow-up task instead.
Verification: go build ./... and -tags server both clean; go vet and
gofmt clean on touched files; go test -race on internal/index,
internal/server (CI skip regex, both bare and -tags server variants),
internal/httpapi, internal/contracts, cmd/mcpproxy, internal/config,
internal/oauth, internal/storage, internal/serveredition/* all green;
golangci-lint v2 (bare and --build-tags server) on touched packages
shows only 6 pre-existing staticcheck issues in test files this PR
never touched; frontend npm ci + npm run build (vue-tsc + vite) +
npx vitest run all green (147 files / 1393 tests, package-lock.json
unchanged); swift test full suite green except one known
environment-caused failure (AppLifecycleTests, caused by a real
~/.mcpproxy/tray-lifecycle.jsonl already present on this shared
machine from outside this session — unrelated to any file this PR
touches, reproduces identically before any of these changes).
Not pushed; branch left detached per instructions.
Verified each round-2 finding against current source before touching
anything; fixed the genuine ones test-first, deferred one with reasons
below. No false positives to reject this round.
FIXED:
1. (high) Repositories.vue: closeAddRegistry()/closeDeleteRegistry() are
wired as useDialogOpen's native-close handler (Escape/backdrop), but
their own addingRegistry/deletingRegistry guard silently skipped the
showAddRegistry/showDeleteRegistry flip. A native close already happened
in the DOM by the time that fires and cannot be undone, so the guard just
desynced Vue state from an already-closed dialog — reopening became a
no-op forever (round 1's H4 bug, reintroduced by round 1's own fix). Split
the native-close handler from the Cancel-button handler; the Cancel/
Delete-cancel buttons keep the guard (now via :disabled) since an
explicit click CAN be blocked mid-submit.
2. (medium) OnboardingWizard.vue: dismiss() (also wired as useDialogOpen's
onClose) awaited up to three sequential engagement-bookkeeping calls
before emit('close'). props.show stayed true for that whole window, so
reopening the wizard from the sidebar Setup entry was a no-op. emit
synchronously first; the bookkeeping now runs decoupled in the
background.
3. (low) ConnectModal.vue: close() (same onClose role) reset several result/
preview fields but not disconnectTarget, so Escape during the "Disconnect
X?" confirm sub-panel left it stale for the next open.
4. (medium-high) internal/runtime/lifecycle.go: annotations_json is only
written at (re)index time, but the reindex trigger is a Hash comparison
that deliberately EXCLUDES annotations (see calculateToolApprovalHash's
own comment in tool_quarantine.go: annotations are unstable across
reconnections, and hashing them in before caused false
"tool_description_changed" spam on every reconnect). So an
already-indexed, otherwise-unchanged tool could never pick up newly
observed annotations short of a manual reindex. Fixed WITHOUT touching
Hash/change-detection (which would reintroduce that exact regression):
applyDifferentialToolUpdate now tracks annotation-only diffs separately
and silently refreshes just those documents (no approval/quarantine
state, no "Tool schema changed" log, no hash rewrite). Covered by
TestApplyDifferentialToolUpdate_BackfillsAnnotationsForUnchangedTool and
...IsIdempotent.
6. (low-medium) RegistryModels.swift: RegistryAddServerErrorBody decoded a
`message` key, but writeRegistryAddError (internal/httpapi/server.go)
serializes the field as `error` — every other error envelope in this API
uses the same shape. err?.message was therefore always nil against the
real backend, so a failed Add-from-Registry always showed the generic
"HTTP 400: ..." text instead of the actual reason. Fixed the CodingKeys
mapping and added a test that asserts on the decoded message text (the
round-1 test used a stub keyed `message`, which "passed" without ever
checking the value — a green test over a broken decode).
7. (low) Same files: round 1's own comment/test claimed the backend
"de-duplicates on conflict" for a name collision. It doesn't —
AddServer (internal/server/server.go) hard-fails with duplicate_name; no
rename/suffix logic exists anywhere in that path. Corrected the
misleading comments in RegistryModels.swift and ServerBrowseView.swift,
and reworked the test's success-case stub to model the one case that IS
real (empty entry name falls back to the entry id) instead of an
invented de-dup suffix.
8. (medium) Settings.vue: hasServerEdition depends on systemStore.status,
populated asynchronously by App.vue's SSE connection in its own
onMounted. On a reload of /settings?tab=teams, Settings.vue's onMounted
could run its `?tab=` membership check before that status frame lands,
silently dropping `teams` with no retry once it arrives. Added a
one-shot retry gated on hasServerEdition, guarded so it never overrides
a tab the user picks in the meantime and is a no-op when the edition
really is personal.
DEFERRED (real, not a false positive, but out of scope for a quick-wins
fix round):
5. (low) internal/index/bleve.go: a pre-105/pre-this-PR on-disk index opens
with its OWN persisted mapping, which never declared annotations_json —
so that field falls to Bleve's dynamic-mapping default (Index=true,
contributes to _all) on a legacy index, while a freshly created index
gets the explicit Index=false mapping added in round 1. Confirmed real:
this is genuine BM25 divergence between legacy and fresh indexes.
NOT fixed here: the obvious-looking fix (disable dynamic mapping) is
actively wrong — output_schema_json has NO explicit field mapping at all
and already relies on the same dynamic-mapping fallback in every index,
old and new alike; disabling it would silently stop storing that field
on every newly created index. A correct fix needs an actual Bleve
mapping-version/migration mechanism (there is none today — the
OutputSchemaHashSchemaVersion precedent lives entirely in the BBolt
approval-record store, not the search index), which is a properly scoped
follow-up, not a round-2 quick fix.
Verification: go build ./... and -tags server; go test -race
./internal/runtime/... (192s, clean); golangci-lint v2 bare and
--build-tags server (only pre-existing, unrelated issues, e.g.
event_bus.go:887 reflect.Ptr); frontend npm run build + npx vitest run
(151 files, 1401 tests, all green; package-lock.json unmodified); swift
test (1179 tests, 1 failure — AppLifecycleTests.
testTheSharedJournalNeverWritesToTheRealInstanceRootUnderTests, a
documented pre-existing environmental flake tied to a real
~/.mcpproxy/tray-lifecycle.jsonl on this machine, unrelated to this diff
and already failing on main).
Verified each round-3 finding against current source before touching anything; fixed the genuine one test-first, confirmed one is already covered by existing tests, and reaffirmed one deferral. No false positives to reject this round — all three findings described the code accurately. FIXED: 1. (medium, T011a) scripts/test-api-e2e.sh's cleanup trap used a blanket `pkill -f "mcpproxy.*serve"` / `pkill -f "launcher-server.*--port 39933"`. Round 1 rejected this same finding on the premise that no tasks.md substantiates it being an in-scope task for this PR; that premise was false — specs/109-ux-navigation- consistency/tasks.md (present on the sibling 109-ux-navigation- consistency branch, both branched from the same commit) lists T011a explicitly under "Phase 2: PR 109-a — quick-wins". Live-verified the actual risk before fixing: this machine had a real tray-managed `mcpproxy serve` (PID 15758) and a second, unrelated worktree's test instance running at the same time — the unfixed script's blanket pkill would have killed both. Replaced both blanket kills with `descendant_pids()`, which snapshots the actual OS-process descendants of $MCPPROXY_PID (recursively, via `pgrep -P`) BEFORE touching it, then reaps only those specific PIDs (and only if still alive) as a fallback for the launcher-test fixture getting orphaned before mcpproxy's own graceful-shutdown reap path runs. Added scripts/test-api-e2e-cleanup-check.sh (the check T011a specifies): starts a decoy `mcpproxy serve` on another port with a scratch data dir, runs the full E2E script, and asserts the decoy is still alive afterward. Ran it for real: 65/66 sub-tests passed (the one failure is the pre-existing, unrelated "server-edition binary present" check, which needs a separately built ./mcpproxy-server and fails identically on main), and the decoy — plus this machine's real tray core — survived the run. 2. (low-medium, T007 Playwright half) e2e/web-ui-sweep/ visual-a11y-sweep.spec.ts had no browser-level proof that the H4 z-index fix (frontend/src/assets/z-index.css) holds under real layout and paint, only jsdom token-ordering coverage. Added "mobile drawer sidebar paints above the sticky header, not under it (H4)": opens the mobile drawer against a real launched instance and asserts via `elementFromPoint` that the drawer, not the header, is the topmost element at a point they both occupy. Verified it actually catches the regression it targets, not just a code read: temporarily re-inverted --z-header/--z-sidebar back to round-1's shipped bug, rebuilt, and reran — the new test timed out failing exactly as expected; restored the tokens and reran the full 25-test suite clean. One real finding surfaced while building this: daisyUI's drawer opens via a CSS `visibility`/`opacity` transition (`allow-discrete`, ~0.2-0.3s), so `elementFromPoint` checked on the very next frame after the checkbox flips can still legitimately see the header for a couple of frames — normal opening animation, not the H4 regression (a permanently wrong settled z-index). The assertion polls for the settled state instead of asserting immediately, so it does not flake on that transition. ALREADY COVERED (not a gap, no change needed): - The finding's other T007 half — "a vitest asserting none of Repositories.vue's three dialogs still uses the `:open` binding" — already exists: frontend/tests/unit/z-index-scale.spec.ts (added in the original PR commit 8bc8d23de, kept through round 1) has both "Repositories.vue dialogs use showModal()/close(), not :open (FR-055)" (lines 71-88, checking exactly the `:open` binding and all three data-test ids) and a broader "every <dialog class=modal> drives its open state imperatively" sweep across all 8 affected components, Repositories.vue included. Wrote a duplicate test file first, then found this and deleted it rather than ship two guards for the same invariant. DEFERRED (confirmed real, reaffirming round 2's deferral — not fixed here): 3. internal/index/bleve.go: re-verified the pre-105/pre-this-PR legacy-index dynamic-mapping divergence against current source. annotations_json still has no on-disk mapping-version guard, RebuildIndex() is still a documented no-op, and output_schema_json still has no explicit field mapping of its own — so it's still true that disabling dynamic mapping to close this gap would silently stop indexing output_schema_json on every freshly created index, on top of every index that already exists today. Round 2's reasoning holds exactly as written: a correct fix needs an actual Bleve mapping-version/migration mechanism, which is properly scoped design work, not a quick-wins round change. Flagging a follow-up task for that design work separately rather than leaving it only in this commit message. Verification: go build ./... and -tags server (-o /dev/null, per the server-tags-build-overwrite gotcha) both clean. No Go, frontend/src, or native code touched this round, so go test -race / vitest / swift test are unaffected by this diff; ran golangci-lint v2 anyway (bare and --build-tags server) — 16 and 19 pre-existing issues respectively, none in any file this round touched (reflect.Ptr inlining, deprecated TopK/Features test usage, deprecated ecdsa.PublicKey.X/Y, ParseDir deprecation — all pre-existing on files untouched by any of the three rounds). scripts/test-api-e2e-cleanup-check.sh and the new Playwright test were both run for real against a locally built instance, not just read. Not pushed; branch left detached per instructions.
Round-4 zcode findings on the E2E cleanup trap (T011a) and a stale Tools.vue stat-card bug. Confirmed genuine, fixed test-first: - F3 (medium): descendant_pids had no test of its own. Extracted it to scripts/descendant-pids.sh (sourced by test-api-e2e.sh) and added scripts/descendant-pids.test.sh, a hermetic unit test that builds a real 3-level process tree and proves the recursive walk reaches a grandchild and great-grandchild, not just direct children. Also extended test-api-e2e-cleanup-check.sh to assert a REAL orphan of the run (the launcher-test fixture, left running by test_launcher_lifecycle Step 5) is actually gone after cleanup, not just that the decoy survives. Wired both scripts into the Makefile (test-descendant-pids, test-e2e-cleanup-check). - F4 (medium): the audit_log sub-test's AUDIT_PID/AUDIT_DATA_DIR/ AUDIT_JSONL_DIR/AUDIT_SERVER_LOG were only reaped on its own fall-through path; an early exit while that sub-test is running leaked them. cleanup() now reaps them too, idempotently. - F1 (medium, partial fix): the descendant snapshot was taken once, before sending the kill signal, missing a child spawned during mcpproxy's own graceful-shutdown window. Now re-snapshotted on every poll of the wait loop while mcpproxy is confirmed still alive. The case where mcpproxy has already exited (crash/OOM) before cleanup() even runs is not fixable without reintroducing the system-wide pattern match T011a deliberately removed (kills unrelated mcpproxy instances) — documented as an accepted, narrow residual gap. - F2 (low): the reap loop now records each descendant's command name at snapshot time and re-checks it before SIGKILL, so a PID reused by an unrelated process in the snapshot-to-reap window is skipped instead of killed. - F6 (low): test-api-e2e-cleanup-check.sh's decoy port was a fixed default (only DECOY_DIR was made unique), colliding between concurrent runs. Now picks an OS-assigned free port. - F7 (low): added --max-time to the two curl calls in test-api-e2e-cleanup-check.sh that lacked it, matching every curl call in test-api-e2e.sh. - Tools.vue activeStatCard (low, flagged rounds 1 and 2, never actually fixed): it only inspected filterStatus, so an approval-only filter (filterStatus empty) still rang the Total stat card as active even though the table was filtered. Total now reads as active only when no filter narrows the table. New regression test: tools-active-stat-card-approval-only.spec.ts (reproduced the bug first, then fixed). F5 (EXIT trap not firing on SIGINT) does not hold up under a faithful reproduction: sending SIGINT to the actual process group (what a terminal's Ctrl-C really delivers) already ran cleanup() correctly on the pre-round-4 code, because the foreground child dies from the same signal, bash's `wait` unblocks, and an untrapped terminating signal still fires the EXIT trap. Verified this directly (group-wide kill -INT and kill -TERM both fired the trap on the unmodified script; only a single-PID kill -INT, which is not what a terminal sends, appeared to hang, and that appearance was itself an artifact of test harness backgrounding setting SIGINT to ignore). Kept explicit `trap ... INT TERM` handlers anyway as harmless, idiomatic defense-in-depth for a supervisor that signals only this process's PID, with an idempotent cleanup() (CLEANUP_DONE guard) so it cannot double-run. Verified locally: go build ./... clean (no Go files touched by this commit); golangci-lint v2 (bare and --build-tags server) shows only pre-existing unrelated findings; frontend build + full vitest run (152 files / 1402 tests) green; scripts/descendant-pids.test.sh green; a full scripts/test-api-e2e-cleanup-check.sh run against a real built mcpproxy/mcpproxy-server passed all 70 E2E tests, confirmed the decoy instance survived on a freshly-picked port, and confirmed the real launcher-server orphan was reaped.
Verify each round-6 finding against the code before fixing; fix genuine
ones test-first, reject/defer two that are out of scope here.
1. internal/index/bleve.go (finding 1, high): an index created before the
annotations_json field mapping existed keeps its old mapping forever
after bleve.Open (no SetMapping in vendored bleve v2.6.1), so the
differential-update backfill's annotations JSON gets indexed as free
text via bleve's dynamic-field defaults, leaking into the field-less
`_all` composite field and skewing BM25 ranking on any upgraded (not
freshly installed) deployment. A full fix needs index-mapping-version
tracking plus rebuild-from-storage, which doesn't exist yet
(RebuildIndex is a stub) -- flagged as a follow-up task instead of
building that here. Applied a bounded, tested mitigation:
warnIfMappingPredatesAnnotations logs an actionable warning on open;
toolMapping.Dynamic = false closes the whole bug class for indexes
created from now on; and an explicit output_schema_json field mapping
(previously undeclared, relying on the same dynamic fallback) closes a
second, pre-existing instance of the same leak that surfaced only once
Dynamic was turned off. New tests: bleve_mapping_migration_test.go.
2. cmd/mcpproxy/tools_cmd.go (finding 4, medium): --tier/--risk/--status/
--approval were only ever applied on the global (no --server) tools-list
path; `--server=x --tier destructive` silently printed every tool
unfiltered. Client-mode (daemon-backed) now reuses
applyGlobalToolFilters, since GET /api/v1/servers/{id}/tools shares
enrichServerTools with the global endpoint and carries the same fields.
Standalone (no-daemon) mode gets a new filterToolMetadataByTier for
--tier (computed locally via contracts.AnnotationTier) and a fast,
explicit error for --status/--approval, which need daemon-persisted
state this path has no access to. A zcode re-review of this fix caught
a residual message bug it introduced: emptying a non-empty standalone
result via --tier fell into the "server doesn't support tools"
diagnostic meant for a server with genuinely zero tools;
standaloneNoToolsMessage now tells the two cases apart.
3. native/macos ServerBrowseView.swift / RegistryModels.swift (finding 2,
medium): "Add to MCPProxy" -> "Added (checkmark) Open" state was tracked
by bare server.id, while the same file's own cross-registry search
dedupe already keys by "registry::id" (catalog ids collide across
registries, MCP-866) -- two colliding cards from different registries
could flip together. Added RepositoryServer.addedKey and switched both
addedServers call sites to it. A zcode re-review found the same
collision class still live in ForEach(results)'s SwiftUI identity and in
addingID; fixed both, plus a stale doc comment that still described the
bug this commit fixes.
4. frontend/src/components/AuthErrorModal.vue (finding 3, medium): the
last dialog left on the old v-if/div modal pattern while its four
siblings were migrated to the browser top layer (FR-055); a 401 raised
while one of those is open painted underneath it. Migrated to a native
<dialog> via the existing useDialogOpen/useModalA11y composables,
deliberately without the click-catcher backdrop form the other dialogs
use (clicking outside must not dismiss this one -- a pre-existing,
audited rule). Replaced the onMounted form-reset with a
watch(props.show), since the dialog no longer unmounts/remounts on
every open/close the way the v-if version did.
5. scripts/test-api-e2e-cleanup-check.sh (finding 5, medium): the T011a
proof's launcher-server pgrep check had no pre-run baseline, so a stray
left by an earlier crashed run or a parallel worktree's own E2E run
produced a false FAIL. Snapshots PIDs before running and diffs with
comm -13 so only a genuinely new PID counts as this run's own leak.
Rejected (not fixed here):
- Finding 6 (specs/109-ux-navigation-consistency/tasks.md T026 vs macOS
DashboardView.swift): tasks.md lives on branch docs/specs-108-109, not
this branch, so it can't be edited from here. The same tasks.md's own
"Follow-ups (not in this spec)" section already lists "a native macOS
Usage view" as deferred future work, directly contradicting T026's
in-scope macOS clause -- a spec/tasks inconsistency for whoever owns
that branch to reconcile, not a reason to invent a new macOS chart
feature inside a quick-wins PR. Flagged as a follow-up task.
Every fix is test-first: internal/index, cmd/mcpproxy and native/macos
each got new regression tests confirmed to fail before the fix (or, for
the macOS swift package which has no such build gate here, verified by
temporarily reverting and re-running) and pass after.
Reviewed with zcode (GLM), the mandated external reviewer for this
branch. It returned a clean pass on internal/index/bleve.go +
cmd/mcpproxy/tools_cmd.go (one residual message bug found and fixed, see
above), a clean pass with three genuine follow-ups on the macOS
ServerBrowseView/RegistryModels change (all three folded into this
commit), and a clean pass with no defects on
scripts/test-api-e2e-cleanup-check.sh. The AuthErrorModal.vue chunk could
not get a completed zcode pass despite five retries over ~20 minutes --
the shared zcode/GLM account was rate-limited (HTTP 429, "Rate limit
reached for requests") by concurrent sibling review sessions running the
same review round in parallel worktrees, not by anything in this diff.
Verified by hand instead, against the identical, already-shipped and
already-tested pattern in AddSecretModal.vue / AddServerModal.vue /
ConnectModal.vue / OnboardingWizard.vue, plus the full frontend build
(vue-tsc + vite) and vitest suite (152 files / 1402 tests, including
auth-single-surface.spec.ts) passing.
Local verification: go build ./...; go test -race on internal/index,
cmd/mcpproxy, internal/runtime (+ subpackages), and an internal/httpapi
tool/tier/search subset; golangci-lint v2 (bare and --build-tags server)
clean on every touched Go package, no new issues anywhere else; npm ci &&
npm run build && npx vitest run (152/152 files, 1402/1402 tests), then
package-lock.json left unchanged (no dependency changes); swift build +
swift test (one pre-existing, unrelated failure confirmed present with
and without this diff: AppLifecycleTests.
testTheSharedJournalNeverWritesToTheRealInstanceRootUnderTests).
Round 7 findings, verified against the code (zcode unavailable this round: 5-hour usage limit, resets 10:35 — reviewed manually instead, same as the finding's own note): - frontend/src/composables/useDialogOpen.ts (high, confirmed): a showModal() dialog's own Escape handling fires a native `cancel` event and, unless that event is canceled, closes the dialog — independently of useModalA11y's document-level keydown listener, whose preventDefault() only affects the keydown event and does not stop the platform's cancel/close algorithm. AuthErrorModal relies on its close() being a no-op while canClose is false, but the native path ignored that gate entirely: Escape closed the <dialog> element regardless, desyncing it from isOpen() (same bricking shape as the existing H4 fix) and defeating the "must not be dismissable" intent the whole modal exists for. Fixed by always preventing the native `cancel` event in useDialogOpen and routing every Escape-driven close through the one close() callback that already owns the permission check, the same way useModalA11y's keydown handler does. Covered by frontend/tests/unit/use-dialog-open-cancel-prevented.spec.ts, confirmed red before the fix (dispatching a synthetic `cancel` event closed the dialog) and green after. - internal/index/bleve.go (medium, confirmed, carried from round 6): round 6 only warned when an on-disk Bleve index predated the annotations_json field mapping, leaving an in-place upgrade with a silently stale mapping (and the resulting free-text/`_all` leakage) until an operator noticed the log line and deleted the directory by hand. Manager.RebuildIndex is a pre-existing unwired no-op stub, not a path to fix this. Replaced the warn-only function with one that closes the stale index, removes its directory, and creates a fresh one with the current mapping automatically — safe because index.bleve is a derived search cache, not a source of truth: an empty index makes every server's tools look newly discovered, and the normal discovery path (applyDifferentialToolUpdate) already reindexes everything as each server reconnects at startup, the same way it backfills a brand-new install. Extended bleve_mapping_migration_test.go with a regression test asserting the rebuilt index carries the current mapping and rejects free-text search on annotations_json, with no operator step. - native/macos/MCPProxy/Views/DashboardView.swift (medium, carried from rounds 5-6): re-scoped rather than implemented. Spec 109 T026's "macOS DashboardView.swift equivalent label" describes the same integer-tick + "Calls to unknown tools" fix web's CallHistogram.vue got, but macOS has no per-tool call-histogram anywhere to attach it to — the file's only chart-like element (Token Distribution) is a per-server token-SIZE bar list with no ticked axis and no "unresolved tool name" concept, a different metric keyed by server name rather than call outcome. Spec 109's own tasks.md lists "a native macOS Usage view" as a Follow-up explicitly "not in this spec" — building one now would be new scope, not a quick win. Added an explanatory comment recording that scope decision next to tokenDistributionSection so this stops being silently re-flagged every round; tracked for the eventual macOS Usage view instead. Verification: go build ./...; go test -race ./internal/index/...; golangci-lint v2 (bare and --build-tags server) clean of new issues; frontend npm ci + vitest run (153 files / 1405 tests green) + vue-tsc build; swift build + swift test (1182 tests, 1 pre-existing failure — AppLifecycleTests.testTheSharedJournalNeverWritesToTheRealInstanceRootUnderTests fails on this machine because ~/.mcpproxy/tray-lifecycle.jsonl already exists from real daily use, unrelated to and unaffected by this comment-only Swift change; reproduces identically without it).
Verify each round-8 zcode finding against the code before fixing; all six
are genuine, fixed test-first, no false positives to reject.
1. internal/index/bleve.go (finding 1, high): the pre-annotations mapping
migration was destructive-before-construct (Close -> os.RemoveAll(path)
-> createBleveIndex(path)), so a failure after the wipe (disk full,
permission error) left the daemon with no index and no way to recover
without operator intervention -- turning a previously-degraded-but-
working deployment (stale mapping, but an openable, functional index)
into a hard outage on every restart. rebuildIfMappingPredatesAnnotations
now builds the replacement at a temporary sibling path FIRST; only once
that succeeds does it retire the original (close, remove, rename the
replacement into place, reopen). If construction fails, the original
index is left untouched and returned as-is, logging the error instead of
failing startup -- the same degraded-but-running outcome round 6 had.
New test: TestRebuildIfMappingPredatesAnnotations_ConstructionFailureKeepsStaleIndexUsable
(forces the failure deterministically via a read-only parent dir).
2. frontend/src/composables/useDialogOpen.ts (finding 2, medium): Escape was
a dead key on the ~8 dialogs (ConnectModal, OnboardingWizard,
Repositories.vue's three dialogs, teams/UserActivity, UserServers,
UserTokens) that pair useDialogOpen alone, with no useModalA11y keydown
listener. handleCancel unconditionally prevented the native `cancel`
event on the assumption a paired keydown listener would call close() --
true only for the ~3 dialogs that pair both composables. handleCancel
now also calls onClose() itself, guarded on isOpen() still reading true
(a no-op, not a double call, when a paired keydown already closed it,
since keydown is synchronous and cancel is a browser-queued task per the
HTML spec). New test dispatches ONLY the native cancel event (no direct
state flip), the case the existing round-7 tests didn't cover.
3. frontend/src/views/ServerDetail.vue (finding 3, medium): App.vue's
<router-view> is keyed on the auth epoch, not the route, so navigating
from one /servers/:serverName to another reuses the same component
instance -- onMounted never reruns, so it never re-read `?tab=`. The
existing props.serverName watch already resets per-server state for
exactly that reason but never touched activeTab, so a stale tab (e.g.
Security) kept showing for the next server even with no ?tab= in its
URL. Factored the onMounted tab-read into readTabFromQuery() and call it
from the watch too, resetting to the default when the new URL carries no
?tab= and honoring it when present.
4. frontend/src/views/Repositories.vue (finding 4, medium): "Added (check)
Open" state (addedServers/addingServerId, Spec 109 FR-063) was keyed by
the bare catalog entry id, while searchServers' own cross-registry dedupe
already keys by `${registry}::${id}` since catalog ids collide across
registries (MCP-866) -- the same bug the macOS half of this PR
(ServerBrowseView.swift's addedKey) already fixed on that side. Added the
matching addedKey() helper on the Web side and switched every
addedServers/addingServerId read and write to it.
5. Makefile / .github/workflows (finding 5, medium): the two new tests
proving the E2E cleanup trap no longer kills unrelated mcpproxy
instances (test-descendant-pids, test-e2e-cleanup-check) were wired into
neither `make test`/`test-e2e` nor any workflow, so a regression
reintroducing the blanket `pkill -f "mcpproxy.*serve"` this PR removes
would pass CI undetected. Wired the hermetic descendant-pids.test.sh into
a new standalone job in unit-tests.yml (no build/binary needed), and the
heavier test-api-e2e-cleanup-check.sh (needs the built binary, re-runs
the full E2E suite as a subprocess) into release-qa-gate.yml's
suite-api-e2e job right after the API E2E suite step, doubling that
job's timeout budget accordingly.
6. cmd/mcpproxy/tools_cmd.go (finding 6, medium): --tier/--risk accepted any
string with no validation; a typo like `--tier destrutive` silently
matched nothing via strings.EqualFold and exited 0 with an empty table --
a CI or audit script grepping for e.g. destructive tools reads that as
"the host has none" instead of "the flag was misspelled". Added
validateTierFilter, checked in runToolsList before any of the three list
paths (global/client-mode/standalone) dispatch; names the actual flag
(--tier vs --risk) in the error since resolvedTierFilter merges them.
Manually verified against a live scratch instance: `--tier destrutive`
now exits 1 with the flag list; `--tier read` still works.
Local verification: go build ./... and -tags server both clean; go test
-race on internal/index and cmd/mcpproxy (touched packages) pass;
golangci-lint v2 (bare and --build-tags server) clean on touched files, only
pre-existing unrelated debt elsewhere; frontend npm run build (vue-tsc +
vite) and full npx vitest run (154 files / 1410 tests) pass; manually built
and ran a scratch mcpproxy instance to exercise the bleve reopen path and the
new --tier validation end to end.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…st gap) Two findings from an external review pass: 1. internal/index/bleve.go (rebuildIfMappingPredatesAnnotations): the finalize step retired the original index directory with os.RemoveAll(indexPath). A partial RemoveAll failure (one permission- impaired or locked leftover file) can delete most-but-not-all of the original and still return an error; on the next startup bleve.Open(indexPath) fails on the mangled remnant, falls through to createBleveIndex(indexPath), and if index_meta.json survived, bleve's O_CREATE|O_EXCL index_meta.Save returns ErrorIndexPathExists — the same operator-intervention outage class round 8 fixed, reachable through a narrower door. Fixed by renaming the original aside (os.Rename) instead of deleting it, with restore-on-failure for both the move-into-place and reopen steps. A zcode follow-up review of this fix flagged the reopen-failure path as the identical gap one step later (very low probability, same shape); hardened it the same way rather than leaving a narrower version of the bug in place. Regression test reproduces the exact partial-delete shape against the real os.RemoveAll implementation (verified: index_meta.json removed, store/root.bolt survives, directory left mangled) and confirms it fails pre-fix, passes post-fix. 2. internal/gatereport/gatereport.go: the "Run E2E cleanup-trap safety check" CI step (suite/api-e2e-cleanup-check, added to close round-8 finding 5) was never added to Manifest(), so deleting that step or typo'ing its --name would leave no fragment at all — nothing reports a manifest entry missing for a name the manifest never expected, and the gate stays green with no evidence the check ever ran. Added EntrySuiteAPIE2ECleanupCheck as a blocking manifest entry, with tests pinning both its presence/blocking-ness and that a missing fragment for it fails the gate. Reviewed with zcode (GLM-5.3), read-only plan mode, per-file chunks. Verdict on both: fix correct, tests genuinely reproduce the bug and fail without it. Additional non-blocking observations from that review: a stale doc line (docs/development/release-gate.md, now updated) and two low-priority/theoretical hardening suggestions for bleve.go left as documented, deferred (a currently-unreachable RemoveAll-error-ignored footgun on future reuse of this rebuild path, and no test coverage for the untestable-without-a-seam restore-on-second-rename-failure branch). Local verification: go build ./... (bare + -tags server), go vet ./..., go test -race ./internal/index/... ./internal/gatereport/... ./cmd/release-gate/..., golangci-lint v2 (.github/.golangci.yml, bare and --build-tags server) clean on touched packages and repo-wide (pre-existing issues elsewhere unrelated to this diff). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two review findings on Spec 109 PR-a (109-a-quick-wins). Both verified
against the real code test-first (regression test written first, shown
red against the pre-fix logic, then green), then cross-reviewed with
zcode.
- frontend/src/views/Tools.vue: activeStatCard's
`if (filterApproval.value) return null` guard (added round 4 to stop
Total misreporting active on an approval-only filter) ran BEFORE the
filterStatus checks, so it also swallowed 'enabled'/'disabled'. With
an approval filter set, clicking the Enabled/Disabled stat card still
applied filterStatus (the table did narrow further) but the card
never rendered active, and a second click re-ran the same
`filterStatus.value = card` no-op instead of toggling off — a
permanently inert control with no visible feedback. Reordered the
checks so filterStatus === 'enabled'/'disabled' wins regardless of
any additional approval filter; the filterApproval-only null (round
4's actual fix target) still applies only when filterStatus is
empty. New test: tools-enabled-stat-card-with-approval-filter.spec.ts.
- frontend/src/views/Settings.vue: the FR-016 `?tab=` sync and the
pre-existing `?focus=` deep link were both applied only in
onMounted. The header's ModeSwitcher (rendered on every page,
including /settings) links to `/settings?focus=routing_mode`;
clicking it while already on /settings is a query-only navigation
reusing the mounted component instance (App.vue keys <router-view>
on the auth epoch, not the route), so onMounted never reruns and the
click did nothing — the same bug class this PR already fixed for
ServerDetail.vue (round-8 finding) via a route-query watcher. Added
the same pattern here: a watch on [route.query.tab,
route.query.focus] that re-applies the tab / re-runs focusField on
change.
A first zcode pass on that fix found one more genuine (minor) gap:
comparing only `focus !== prevFocus` left a REPEAT click of the same
ModeSwitcher link a dead click once the user had manually switched
Settings tabs in between (same focus value, tab drifted away).
ModeSwitcher's RouterLink is a bare `/settings?focus=routing_mode`
with no `tab` key, so following it always replaces the whole query —
`tab` reverting from a real string to undefined is exactly that
pattern, and is distinct from watch(activeTab)'s own
`{ ...route.query, tab }` writeback, which never unsets `tab` — so
the signal cannot false-positive on our own self-caused navigation.
Added that second condition and a third regression test for the
repeat-click-after-manual-drift scenario. A second zcode pass on the
amended fix returned no further findings. New/updated test:
settings-query-only-nav-focus.spec.ts.
Verification: go build ./... clean (no Go files touched); go vet
./internal/... and ./cmd/... clean; golangci-lint v2 (bare and
--build-tags server) on internal/httpapi and internal/server shows only
7 pre-existing staticcheck findings in test files this change never
touched; go test -race on internal/config, internal/contracts,
internal/oauth, internal/storage and internal/httpapi (server tag, CI
skip regex) all green; frontend npx vue-tsc --noEmit clean; npm run
build clean (package-lock.json unchanged); npx vitest run green (156
files / 1414 tests, up from 1411). Both fixes cross-reviewed with
zcode (GLM-5.3, read-only plan mode): Tools.vue fix verdict NONE first
pass; Settings.vue fix verdict NONE on the second pass after the
repeat-click gap found on the first was fixed.
…eal RebuildIndex An index opened with bleve.Open keeps the mapping it was created with. Indexes created before annotations_json / output_schema_json were mapped explicitly fall back to bleve's dynamic defaults, which full-text index both fields into `_all`, so the field-less MatchQuery in SearchTools ranked identical corpora differently depending only on when the index was created. Round 7 wiped such an index and waited for rediscovery; RebuildIndex was a logging no-op. - Staleness detection on open (indexStaleReason): the persisted `_mapping` bytes are compared with the current mapping (catches every future mapping change with no manual bump), plus an indexSchemaVersion stamp for changes in how documents are derived (searchable_text). - RebuildIndex is real: reads every document's stored fields (sorted, paginated), re-derives searchable_text, writes a current-mapping index in <path>.rebuild, stamps the version last, then swaps it in. The index is its own source: storage keeps no annotations or index hash, and copying stored fields keeps full_tool_name byte-for-byte (SC-005). - Crash safety: index_meta.json is removed first and moved in last; a directory left without it is recreated empty on the next open, and a leftover .rebuild dir is removed. The nested profiles/ directory under the shared index is never touched (round 7's RemoveAll deleted it). - createBleveIndex split into currentIndexMapping() + create/stamp; output_schema_json and annotations_json stay explicit stored-only fields under Dynamic=false. Tests: legacy index migrated with all fields kept and scores equal to a fresh index; annotations_json not field-less matchable on fresh, migrated and runtime-rebuilt indexes (with a precondition proving the legacy leak); output_schema_json stored/returned on fresh and migrated indexes; version-only staleness; profile dir preserved; interrupted swap recovery; stale rebuild dir cleanup; Manager.RebuildIndex keeps docs, scores and writability; pagination past one page. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…build zcode round 1: a swap or reopen failure after the old index was closed left b.index nil, so every later call (Close included) panicked. Recover by recreating an empty current-mapping index in place (discovery re-populates it) and still return the error; a failed cleanup of the .rebuild dir after a completed swap is now a warning. Close is nil-safe. Also corrects newBleveIndexAt's comment: profile indexes live under index.bleve/profiles/<slug>/, the only entry a shared-index rebuild keeps. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rebasing onto 109-a-quick-wins brought rounds 8-9, which hardened the now-replaced rebuildIfMappingPredatesAnnotations. Their two guarantees are kept in the new mechanism: - Round 8: a failed migration no longer fails startup. newBleveIndexAt logs and keeps serving the stale-mapping index (failure before the swap) or the empty recovered one (failure after it); the next open retries. It only errors if no index is left at all. - Round 9: an undeletable file inside store/ must not strand the swap. retireIndexEntries renames bleve's entries to .retired-* names in the same directory (index_meta.json first) and only then best-effort deletes them; leftovers are retried on the next open. The rename stays in the same parent because moving a directory elsewhere also needs write access to it (its ".." entry), which is exactly what a permission-impaired store/ lacks. Round 9's rename-aside of the whole index directory also moved and then deleted the nested profiles/ directory; retiring entries individually keeps it. Ported both regression tests, plus the same unremovable-store case for interrupted-swap recovery. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Deploying mcpproxy-docs with
|
| Latest commit: |
89bafad
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d9f4d3c6.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://claude-nostalgic-bell-8dfa1a.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
CI (macos-15 runner) failed TestNewBleveIndexAt_MigrationSurvivesUnremovableStore and ...InterruptedSwapRecoverySurvivesUnremovableStore: that macOS refuses to rename a non-writable store/ even within its own parent directory (macOS 26 allows it, so it passed locally). Same-directory renames of individual entries therefore cannot route around a permission-impaired store/. retireIndexEntries now renames the whole index directory to a <dir>.retired-<n> sibling (atomic, needs write access to the parent only, the approach PR 1378 round 9 used), recreates dir, and moves profiles/ back so per-profile indexes survive; if that move fails, everything is put back instead. A crash between the rename and the move is repaired on the next open: a retired leftover that still holds profiles/ returns it before being deleted (new test). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The shared index directory holds profiles/, and the rebuild now renames that directory as a whole. Windows cannot rename a directory with open files inside, and on POSIX a profile write during the rename would hit a missing path. Root Manager.RebuildIndex closes and drops open profile indexes first (as DropProfile does); ForProfile reopens them lazily. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 36252860325 --repo smart-mcp-proxy/mcpproxy-go
|
…-8dfa1a # Conflicts: # frontend/src/components/OnboardingWizard.vue # internal/index/bleve.go # internal/index/bleve_mapping_migration_test.go # oas/docs.go
A Close() error on the old index during RebuildIndex returned early with
b.index still pointing at that handle. bleve's scorch backend tears an
index down internally (closing its internal channel, draining async
tasks) before the error-prone part of its own Close runs, so the old
handle is torn down whether or not Close returns an error. Leaving it
installed served requests off a dead index and risked a double-close
panic ("close of closed channel") the next time the index was closed.
RebuildIndex now logs a Close() error and proceeds to swap in the
already-built replacement instead of discarding it and bailing out with
the dead handle in place.
Added TestRebuildIndex_OldIndexCloseFailureStillSwaps, which wraps a real
index so Close tears it down but still reports an error, confirming the
migration completes and the swapped-in index is live (further writes and
Close succeed).
Review round 1 (zcode), finding F1.1: internal/index/bleve.go RebuildIndex.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
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.
What
internal/index/bleve.gonow versions the Bleve index mapping and migrates old indexes, andRebuildIndexactually rebuilds.indexStaleReason): Bleve saves the index's mapping as JSON (internal key_mapping) when it creates the index. On open, that saved copy is compared byte-for-byte withcurrentIndexMapping(), so any future mapping change is caught without a manual version bump.indexSchemaVersionstamp covers changes to how documents are built (e.g.searchable_text), which the mapping can't show.RebuildIndexis real (it was a logging no-op):searchable_textthrough the samesearchableText()helpertoolDocumentuses.<path>.rebuildand stamps the version last.full_tool_nameunchanged, so scores don't move (SC-005).annotations_jsonandoutput_schema_jsonare explicit stored-only fields, andDynamic=falsestays.Why
bleve.Openkeeps whatever mapping the index was created with. On indexes created before these two fields were mapped, Bleve's dynamic defaults indexed both into_all. The field-lessMatchQueryinSearchToolstherefore ranked the same corpus differently depending only on when the index was created. #1378's review rounds 2–3 confirmed this and deferred it.What this replaces from #1378 (rounds 6–9)
rebuildIfMappingPredatesAnnotationsis removed. Its guarantees are kept:store/that can't be deleted can't strand the swap. The old index directory is renamed as a whole to a<dir>.retired-<n>sibling, which is atomic and needs write access to the parent only. The directory is then recreated and the delete afterwards is best-effort. Renaming entries inside the directory doesn't work: the macOS 15 CI runner refuses to rename a non-writablestore/even within its own parent.It also fixes a gap in that function: moving the whole
index.bleveaside and deleting it removed the nestedprofiles/directory holding the per-profile indexes.profiles/is now moved back before the delete, and if that fails, everything is put back. A crash between those two steps is repaired on the next open. RootManager.RebuildIndexalso closes open profile indexes before the rename, because Windows can't rename a directory with open files inside;ForProfilereopens them lazily.Crash safety
index_meta.jsonis taken out first and moved in last. Any interruption leaves either the old index or a directory with no metadata, which the next open recreates empty; discovery then re-populates it..rebuilddirectory is removed on open.Behaviour change
output_schema_jsonis stored and returned but no longer searchable through the field-less query on any index, matchingannotations_json(the round-6 decision). Onmain, output-schema text is still searchable that way on every index.Tests (
internal/index/bleve_mapping_migration_test.go)annotations_jsoncan't be matched by a field-less query on fresh, migrated, or runtime-rebuilt indexes. A precondition check shows the legacy index does leak it, so the test can't pass trivially.output_schema_jsonis explicitly mapped, stored, and returned on fresh and migrated indexes..rebuilddirectory is removed;Manager.RebuildIndexkeeps documents, scores, and writability;store/during interrupted-swap recovery;profiles/restored from a retired leftover after a crash;Mutation checks: with the migration disabled, 6 tests fail. With the round-8 "keep serving" path made fatal, its test fails.
Verification
go test -race ./internal/index/passes.go build ./...andGOOS=windows go vet ./internal/indexare clean..github/.golangci.yml) reports 0 issues, bare and with--build-tags server.-raceruns of runtime, server (CI skip regex), httpapi and appctx passed, before the last two index-only commits.de867e8failed the two unremovable-store/tests on macOS 15, which the whole-directory rename in the follow-up commits fixes.