Replace auto-reindex hooks with staleness signals + background-bootstrap status

[elliottregan]

Summary

• Remove auto-reindex hooks from lefthook.yml — the post-commit, post-checkout, and post-merge hooks that fired cspace-search index on every git event are gone. They were more costly than useful for typical commits.
• Add staleness detection (search/corpus/staleness.go) — cheap checks that compare git state against qdrant without re-embedding. CodeStaleness diffs git ls-files hashes; CommitsStaleness compares commit counts.
• Add index status file (search/status/) — atomic JSON status file at .cspace/search-index-status.json tracking per-corpus state (completed/failed/disabled), in-progress runs with throttled progress, and timestamps.
• cspace search status CLI command — human-readable and --json output showing all corpus states, staleness, and any running index. Added to both cspace search and standalone cspace-search.
• Staleness warnings in query responses — queries now annotate the Envelope.Warning field when the index is stale, across CLI and MCP surfaces.
• search_status MCP tool — structured status output for agents to check index health before high-stakes queries.
• Updated using-semantic-search skill — replaced auto-refresh docs with the new manual workflow: check status, reindex when stale.

Related decision: .cspace/context/decisions/2026-04-22-scope-cspace-search-to-code-commits-close-docs-decisions-con.md

Checklist

• Remove auto-reindex hooks from lefthook.yml
• Add staleness detection (search/corpus/staleness.go)
• Add status file writer + types (search/status/)
• Add cspace search status CLI command (both surfaces)
• Surface staleness warnings in query responses (CLI + MCP)
• Expose search_status MCP tool
• Integrate status file writes into index.Run
• Update using-semantic-search skill
• Unit tests for staleness, status, and MCP

Test plan

• go test ./search/... — all search package tests pass (corpus, status, mcp, index, query, config, cluster)
• go vet ./... — clean
• go build ./... — clean
• Manual: cspace-search status shows correct output for never-indexed, disabled, completed, and stale corpora
• Manual: cspace-search status --json emits valid structured JSON
• Manual: cspace-search index --corpus=code writes status file with correct state and count
• Manual: verify phase 16 background bootstrap still works in a fresh cspace up (requires live container)
• Manual: verify search_status MCP tool returns correct output via cspace search mcp stdio

---

Additional: hook/CI hardening

Audit findings

1. rc: config was broken (root cause Route implementer through supervisor and persist event log #1) — lefthook.yml had rc: export PATH="$HOME/go/bin:$PATH" which is a raw shell command, but lefthook's rc: expects a file path. The generated hook script rendered this as [ -f export PATH="..." ] && . export PATH="..." — a file-existence test on a nonsensical filename. It never matched, so ~/go/bin was never added to PATH, and golangci-lint/goimports were invisible to the hooks.

2. --new-from-rev=HEAD~1 was too narrow (root cause Docs: Scaffold Astro + Starlight project #2) — Pre-commit lint only flagged issues introduced since the prior commit. Pre-existing lint in touched files slipped through, and if a prior commit was made without hooks (due to root cause Route implementer through supervisor and persist event log #1), the delta was against an unchecked baseline.

3. lefthook install was never automated (root cause Docs: Scaffold Astro + Starlight site in docs/ #3) — make setup-hooks existed but was not called by make build, make test, or any commonly-run target. The README didn't mention it. Fresh-clone contributors had no hooks active.

4. CI lint was PR-only (root cause Docs: Getting Started guides #4) — The lint job had if: github.event_name == 'pull_request', so pushes to main (e.g. admin merges) skipped lint entirely. The check job ran vet and test but not fmt-check, letting format regressions through.

5. No --no-verify abuse found — No commits in history used --no-verify.

6. No core.hooksPath override — Git config was clean.

Changes

lefthook.yml + .lefthookrc (new file):

• Extract export PATH="$HOME/go/bin:$PATH" into .lefthookrc; point rc: at the file so the hook script sources it correctly.
• Widen pre-commit lint from --new-from-rev=HEAD~1 to full golangci-lint run ./....
• Add vet-go (go vet ./...) as a parallel pre-commit step gated on *.go files.

Makefile:

• setup-hooks now fails loudly if lefthook is not in PATH (instead of silently succeeding via the fallback chain in the hook script).
• New check-hooks target warns to stderr if .git/hooks/pre-commit is missing; wired into build so contributors notice on first build.

.github/workflows/ci.yml:

• Removed if: github.event_name == 'pull_request' from the lint job — lint now runs on pushes to main too.
• Added Format check step (make fmt-check) to the check job.
• Note for maintainer: mark check, lint, and integration-search as required status checks in repo settings after merge.

README.md:

• Added make install-tools and make setup-hooks to the Development section.
• Added a "Git Hooks" subsection explaining the lefthook setup.

Lint fixups (surfaced by the stricter pre-commit lint):

• internal/cli/search.go — gofmt: removed trailing whitespace in struct tag alignment.
• search/corpus/staleness.go — ineffassign: removed unused headHash variable (the function validates git state via rev-list --count).
• search/mcp/server_test.go — staticcheck SA1012: replaced nil context with context.TODO() in test calls.

🤖 Generated with Claude Code

Addressed review feedback

All items from the review comment are now fixed:

#	Severity	Issue	Commit
1	🔴 Critical	Data-loss race in runSearchInit — outer writer clobbers inner writers	94c8b6d
2	🔴 Critical	CommitsStaleness permanent false positive under commits.limit	3c576c9
3	🟠 Significant	Staleness check costs 100–500ms on every query — added 30s TTL cache	0d6cd6c
4	🟠 Significant	CodeStaleness misses deleted-but-still-indexed ghost entries	494828c
5	🟡 Minor	Duplicate status logic between MCP and CLI — extracted status.Compute	c76d629
6	🟡 Minor	Writer.flush swallows errors silently — added ErrLog callback	c76d629
7	🟡 Minor	30s git ls-files timeout too generous on hot path — reduced to 5s	0d6cd6c
8	🟡 Minor	Redundant time.Now().UTC() calls — capture once per mutator	c76d629
9	🟡 Minor	DisableCorpus flushes unnecessarily — added no-op short-circuit	c76d629

New tests added

• TestWriter_DisableCorpusDoesNotClobberExistingEntries — regression for Route implementer through supervisor and persist event log #1
• TestCommitsStaleness_RespectsLimit — regression for Docs: Scaffold Astro + Starlight project #2
• TestStalenessCache_HitsWithinTTL / TestStalenessCache_ExpiresAfterTTL — cache behavior for Docs: Scaffold Astro + Starlight site in docs/ #3
• TestCodeStaleness_DeletedFiles — ghost detection for Docs: Getting Started guides #4
• TestCompute_Basic — shared Compute function for Docs: CLI Reference #5
• TestWriter_FlushReportsErrors — error surface for Docs: Architecture & Concepts #6
• TestWriter_DisableCorpusShortCircuit — no-op behavior for issue-4: Getting Started guides #9

Durable context decisions logged

5 pattern decisions logged to .cspace/context/decisions/ in 4171564:

1. Writer instances must refresh state before writing OR be single-use
2. Don't compare counts when limits are in play — compare identities or timestamps
3. Secondary work injected into query paths needs a latency budget
4. Delta detection needs symmetric difference, not one-sided membership
5. Don't use planet names for agent-spawned cspace instances

Instance naming guidance

Updated lib/skills/delegating-container-agents/SKILL.md, lib/agents/coordinator.md, and CLAUDE.md to note that planet names are reserved for the TUI — agents should use descriptive names (issue-<n>, cs-agent-<n>, etc.) in 1b3fcd0.

Search bootstrap now opt-in

b1980b5 — Phase 16 (search bootstrap) is now role-aware:

• cspace up: NO auto-index by default; pass --index to opt in
• cspace advisor up: auto-index (session-continuous, search-heavy)
• cspace coordinate: auto-index for the coordinator
• Logged decision Docs: Architecture & Concepts #6: "Bootstrap search indexing is opt-in; advisors and coordinators get it by default"

🤖 Generated with Claude Code

[netlify]

✅ Deploy Preview for cspace-cli canceled.

Name	Link
🔨 Latest commit	cc18f84
🔍 Latest deploy log	https://app.netlify.com/projects/cspace-cli/deploys/69e834ffd4734f0008665402

[netlify]

✅ Deploy Preview for cspace-cli canceled.

Name	Link
🔨 Latest commit	84948bb
🔍 Latest deploy log	https://app.netlify.com/projects/cspace-cli/deploys/69e845e0c8d8a800080fcf25

[elliottregan]

Review: staleness signals + status file

Good structure and clean mapping of commits to tasks. Two blocking bugs before merge, plus some significant-cost concerns and minor polish.

---

🔴 Critical

1. Data-loss race in runSearchInit — context/issues disable wipes prior code/commits state

In internal/cli/search_init.go, the init loop creates an outer status.Writer:

sw, _ := status.NewWriter(root)
for _, corpusID := range []string{"code", "commits", "context", "issues"} {
    err := runSearchIndex(corpusID, true)
    switch {
    case errors.Is(err, config.ErrCorpusDisabled):
        sw.DisableCorpus(corpusID)
    ...

runSearchIndex internally creates its own status.Writer per call and persists Last[code]=completed, Last[commits]=completed. The outer sw captured its in-memory snapshot at iteration start (empty) and never refreshes. When the loop reaches context (disabled) and calls sw.DisableCorpus("context"), the outer writer flushes its stale empty state + the new context entry, clobbering the code/commits entries from disk.

Net effect with default config (code+commits enabled, context+issues disabled): after cspace search init, the status file shows only context: disabled, issues: disabled — the two corpora you actually care about are erased.

Fixes in order of preference:

• Move the disabled-state write into runSearchIndex itself (detect ErrCorpusDisabled from config.Build, write DisableCorpus with a fresh writer, return). Init loop then doesn't need its own sw.
• Or: make status.Writer.DisableCorpus (and every mutator) re-read the file before merging its in-memory delta.

Either path needs a regression test that exercises the full init sequence and asserts all four corpora have the expected state afterward.

---

2. CommitsStaleness permanently reports false positives when commits.limit is set

search/corpus/staleness.go::CommitsStaleness compares rev-list --count HEAD against len(existing). With the default commits.limit: 500 and any repo > 500 commits, repoCount - indexedCount is never zero — so every query emits "N new commits since last index" forever, even immediately after a fresh index. The warning becomes meaningless noise and erodes trust in the signal.

Fix: compare max indexed commit date to git log -1 --format=%cI HEAD. The commits payload already carries a timestamp; scroll once for the max, compare, emit staleness only if HEAD > maxIndexed. A test with limit: 2 on a 5-commit repo would catch this.

---

🟠 Significant

3. Staleness check is on the MCP search hot path, costs 100–500ms per query

mcpAppendStalenessWarning (called from every search_code/search_commits MCP query) runs CodeStaleness or CommitsStaleness unconditionally. CodeStaleness does: full qdrant ExistingPoints scroll + git ls-files + read + SHA256 every tracked file. On this repo (~1265 files) that's a few hundred ms per query. Agents doing 5–10 mid-turn queries pay seconds of invisible overhead.

Options:

• Cache the result with a short TTL (say 30s) keyed on corpus+projectRoot. Accept that staleness accumulating during the cache window doesn't surface immediately — the warning is advisory anyway.
• Timebox it — if the check doesn't return within 100ms, skip the warning for this call.
• Move staleness off the query path entirely and rely on search_status as the explicit agent-facing check. The skill update already points agents there; that might be enough.

My preference is the cache, with a test asserting the second call within the window doesn't touch git/qdrant.

---

4. CodeStaleness misses deleted-but-still-indexed files

The check only counts files whose current hash is absent from the index; it doesn't count ghost entries (files in the index whose paths are no longer tracked). After a significant delete, the index is stale in a way this signal won't surface. Cheap fix: after the git ls-files walk, compare len(existing) to len(tracked files that passed the filter); add the delta to changed with a qualifier in the Reason.

---

🟡 Minor

• 5. Duplicate status logic between MCP and CLI. handleStatus in search/mcp/server.go and runSearchStatus in internal/cli/search.go compute the same shape. Extract a status.Compute(cfg, projectRoot) that both surfaces consume — keeps the MCP output schema and the CLI --json output guaranteed-identical.
• 6. Writer.flush silently drops errors. If the tmp write or rename fails (disk full, perms), we lose the status update without any signal. Wire through a stderr log or an error return so the operator can see "status file write failed".
• 7. 30-second timeout on CodeStaleness's git ls-files is too generous given this runs on the query path. 5s max, or propagate the caller's context deadline.
• 8. Redundant time.Now().UTC() calls inside status.Writer mutators — capture once per call.
• 9. DisableCorpus flushes per-corpus in the init loop — small but multiplied by bootstrap frequency. Consider a Flush() control or skip when state unchanged.

---

✅ Things I liked

• MCP wrapper flattening preserved (no regression of the {\"clusters\":{\"clusters\":[]}} bug from before).
• ErrCorpusDisabled sentinel reused cleanly from the Make corpus indexing configurable per project; disable GitHub issues corpus by default #60 work.
• Atomic write-tmp-rename on status file — no partial reads.
• Test coverage for the happy paths of both staleness functions and the status writer.
• Skill update reads well — the "check status → reindex if stale → query" workflow is exactly right.

---

🧪 Test gaps to cover before merge

• TestCommitsStaleness_WithLimit — regression test for the limit bug.
• TestRunSearchInit_PreservesAllCorporaState — regression test for the data-loss race.
• TestWriter_ConcurrentWrites — two writers racing on the same file; verify no data loss.
• TestStaleness_CacheHit (once the cache lands) — second call within TTL makes no git/qdrant calls.

---

Summary

Ship-blockers: #1 (data loss) and #2 (permanent false positive). The rest are improvements.