Commit5d13cf5
fix(search): repos that arrive without a push were never indexed
fix(search): repos that arrive without a push were never indexed indexChangedFiles() is called from exactly ONE place — the git post-receive hook. Correct for pushes, wrong for every other way a repository comes into existence: - bulk import CLONES; git clone fires no post-receive hook - POST /new + first-repo-scaffold writes via the git plumbing API - forks copy the bare repo directly So both onboarding paths shipped earlier in this session produced repositories where gluecron_semantic_search, repo chat and "ask this repo" returned NOTHING. Silently — an empty index is indistinguishable from "no matches", which is exactly why it survived. It surfaced while wiring the agent-first path, which depends entirely on that index being populated. indexExistingRepo() walks the tree at HEAD (post-receive has a changed-paths delta; a repo that just arrived has none) and hands the result to the same indexChangedFiles() the hook uses. Filtering, the per-push cap and the embedding model stay in semantic-index.ts — re-implementing them here would let scaffolded, imported and pushed repos drift apart in what they index. Wired into both onboarding paths, and the bulk-import report now shows files indexed alongside secrets and security findings. Bounded and defensive: vendor/build directories skipped, depth capped so a pathological monorepo cannot turn one import into a thousand tree reads, concurrency bounded so importing 100 repos does not open 100 concurrent walks. A single unreadable directory does not abandon the index, and the whole thing returns an error field rather than throwing — an unindexed repo is degraded search, not a failed import. Deps injected rather than mock.module()'d on ../git/repository, which is imported by dozens of test files and leaks a global mock across the run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5 files changed+371−05d13cf558c52c73169bd5c706f0679198e4cc511
5 changed files+371−0
Addedsrc/__tests__/index-existing-repo.test.ts+161−0View fileUnifiedSplit
@@ -0,0 +1,161 @@
1/**
2 * Repositories that arrive without a push must still be indexed.
3 *
4 * indexChangedFiles() is called from exactly ONE place — the git post-receive
5 * hook. That is correct for pushes and wrong for every other way a repository
6 * comes into existence:
7 *
8 * - bulk import CLONES (git clone fires no post-receive hook)
9 * - POST /new + first-repo-scaffold writes via the git plumbing API
10 * - forks copy the bare repo directly
11 *
12 * So both onboarding paths built in this session produced repositories where
13 * gluecron_semantic_search, repo chat and "ask this repo" returned nothing.
14 * Silently — an empty index is indistinguishable from "no matches", which is
15 * why it survived. It was found while wiring the agent-first path, which
16 * depends entirely on that index being populated.
17 *
18 * Deps are injected rather than mock.module()'d: ../git/repository is imported
19 * by dozens of test files and a module mock leaks across the whole run.
20 */
21
22import { describe, expect, it } from "bun:test";
23import { readFileSync } from "fs";
24
25const SRC = readFileSync("src/lib/index-existing-repo.ts", "utf8");
26
27type Entry = { name: string; type: string; path?: string };
28
29function treeFrom(map: Record<string, Entry[]>) {
30 return async (_o: string, _r: string, _ref: string, path?: string) =>
31 map[path ?? ""] ?? [];
32}
33
34describe("tree walking", () => {
35 it("collects blobs across nested directories", async () => {
36 const captured: string[][] = [];
37 const { indexExistingRepo } = await import("../lib/index-existing-repo");
38 const r = await indexExistingRepo(
39 { repositoryId: "r1", owner: "a", repoName: "b", ref: "main", commitSha: "sha" },
40 {
41 getTree: treeFrom({
42 "": [
43 { name: "index.ts", type: "blob", path: "index.ts" },
44 { name: "src", type: "tree", path: "src" },
45 ],
46 src: [{ name: "app.ts", type: "blob", path: "src/app.ts" }],
47 }),
48 indexChangedFiles: async (a) => {
49 captured.push(a.changedPaths);
50 return { indexed: a.changedPaths.length, skipped: 0, model: "test" };
51 },
52 }
53 );
54 expect(r.indexed).toBe(2);
55 expect(captured[0].sort()).toEqual(["index.ts", "src/app.ts"]);
56 });
57
58 it("skips vendor and build directories", async () => {
59 let seen: string[] = [];
60 const { indexExistingRepo } = await import("../lib/index-existing-repo");
61 await indexExistingRepo(
62 { repositoryId: "r1", owner: "a", repoName: "b", ref: "main", commitSha: "sha" },
63 {
64 getTree: treeFrom({
65 "": [
66 { name: "node_modules", type: "tree", path: "node_modules" },
67 { name: "dist", type: "tree", path: "dist" },
68 { name: "keep.ts", type: "blob", path: "keep.ts" },
69 ],
70 node_modules: [{ name: "junk.js", type: "blob", path: "node_modules/junk.js" }],
71 dist: [{ name: "out.js", type: "blob", path: "dist/out.js" }],
72 }),
73 indexChangedFiles: async (a) => {
74 seen = a.changedPaths;
75 return { indexed: a.changedPaths.length, skipped: 0, model: "test" };
76 },
77 }
78 );
79 expect(seen).toEqual(["keep.ts"]);
80 });
81
82 it("delegates filtering to indexChangedFiles rather than duplicating it", () => {
83 // isCodeFile, the per-push cap and the embedding model all live in
84 // semantic-index.ts. Re-implementing them here would let scaffolded and
85 // pushed repos drift apart in what they index.
86 //
87 // Comments stripped: the source explains this delegation in prose that
88 // names the very symbol being asserted absent.
89 const code = SRC.replace(/\/\*[\s\S]*?\*\//g, "").replace(/^\s*\/\/.*$/gm, "");
90 expect(code).toContain("indexChangedFiles");
91 expect(code).not.toContain("isCodeFile");
92 });
93});
94
95describe("fault isolation", () => {
96 it("never throws when the tree read fails", async () => {
97 const { indexExistingRepo } = await import("../lib/index-existing-repo");
98 const r = await indexExistingRepo(
99 { repositoryId: "r1", owner: "a", repoName: "b", ref: "main", commitSha: "sha" },
100 {
101 getTree: async () => {
102 throw new Error("disk on fire");
103 },
104 indexChangedFiles: async () => ({ indexed: 0, skipped: 0, model: "test" }),
105 }
106 );
107 // A single unreadable directory must not abandon the index, and an
108 // unindexed repo must never fail an import or a repo creation.
109 expect(r.indexed).toBe(0);
110 expect(r.error).toBeUndefined();
111 });
112
113 it("reports an embedding failure instead of throwing", async () => {
114 const { indexExistingRepo } = await import("../lib/index-existing-repo");
115 const r = await indexExistingRepo(
116 { repositoryId: "r1", owner: "a", repoName: "b", ref: "main", commitSha: "sha" },
117 {
118 getTree: treeFrom({ "": [{ name: "a.ts", type: "blob", path: "a.ts" }] }),
119 indexChangedFiles: async () => {
120 throw new Error("embedder down");
121 },
122 }
123 );
124 expect(r.indexed).toBe(0);
125 expect(r.error).toContain("embedder down");
126 });
127
128 it("returns zero for an empty repository without calling the embedder", async () => {
129 let called = false;
130 const { indexExistingRepo } = await import("../lib/index-existing-repo");
131 const r = await indexExistingRepo(
132 { repositoryId: "r1", owner: "a", repoName: "b", ref: "main", commitSha: "sha" },
133 {
134 getTree: treeFrom({}),
135 indexChangedFiles: async () => {
136 called = true;
137 return { indexed: 0, skipped: 0, model: "test" };
138 },
139 }
140 );
141 expect(r.indexed).toBe(0);
142 expect(called).toBe(false);
143 });
144});
145
146describe("both onboarding paths index", () => {
147 it("bulk import indexes each repo", () => {
148 const mig = readFileSync("src/lib/migration-onboarding.ts", "utf8");
149 expect(mig).toContain("indexExistingRepo");
150 expect(mig).toContain("filesIndexed");
151 });
152
153 it("new-repo scaffold indexes after committing", () => {
154 const scaffold = readFileSync("src/lib/first-repo-scaffold.ts", "utf8");
155 expect(scaffold).toContain("indexExistingRepo");
156 // Must come after the commits — there is nothing to index before them.
157 expect(scaffold.indexOf("createOrUpdateFileOnBranch")).toBeLessThan(
158 scaffold.lastIndexOf("indexExistingRepo")
159 );
160 });
161});
Modifiedsrc/lib/first-repo-scaffold.ts+27−0View fileUnifiedSplit
@@ -32,6 +32,8 @@ export interface ScaffoldResult {
3232 workflowsSynced: number;
3333 /** CI runs enqueued — this is what makes the repo show a green check. */
3434 runsEnqueued: number;
35 /** Code files embedded, so semantic search works from the first minute. */
36 filesIndexed: number;
3537 errors: string[];
3638}
3739
@@ -142,6 +144,7 @@ export async function scaffoldFirstRepo(
142144 workflowCommitted: false,
143145 workflowsSynced: 0,
144146 runsEnqueued: 0,
147 filesIndexed: 0,
145148 errors: [],
146149 };
147150
@@ -214,5 +217,29 @@ export async function scaffoldFirstRepo(
214217 }
215218 }
216219
220 // Semantic index. Same reason the workflow sync is here: indexChangedFiles()
221 // runs only from the git post-receive hook, and these files were written
222 // through the plumbing API. Without this the repo's semantic search, repo
223 // chat and "ask this repo" return nothing — and an empty index is
224 // indistinguishable from "no matches", so nobody notices.
225 if (headSha) {
226 try {
227 const { indexExistingRepo } = await import("./index-existing-repo");
228 const idx = await indexExistingRepo({
229 repositoryId: opts.repositoryId,
230 owner: opts.owner,
231 repoName: opts.repoName,
232 ref: opts.defaultBranch,
233 commitSha: headSha,
234 });
235 result.filesIndexed = idx.indexed;
236 if (idx.error) result.errors.push(`index: ${idx.error}`);
237 } catch (err) {
238 result.errors.push(
239 `index: ${err instanceof Error ? err.message : String(err)}`
240 );
241 }
242 }
243
217244 return result;
218245}
Addedsrc/lib/index-existing-repo.ts+149−0View fileUnifiedSplit
@@ -0,0 +1,149 @@
1/**
2 * Semantically index a repository that arrived without a push.
3 *
4 * `indexChangedFiles()` is called from exactly one place: the git post-receive
5 * hook. That is correct for the push path and wrong for every other way a
6 * repository comes into existence:
7 *
8 * - Bulk import clones with `git clone`, which never fires post-receive.
9 * - POST /new + first-repo-scaffold writes through the git plumbing API,
10 * which also never fires post-receive.
11 * - Forks copy the bare repo directly.
12 *
13 * So the two onboarding paths built in this session produced repositories
14 * where `gluecron_semantic_search`, the repo chat, and "ask this repo" all
15 * returned nothing — silently, because an empty index is indistinguishable
16 * from "no matches". The agent-first experience depends entirely on that
17 * index being populated, which is how this was found.
18 *
19 * post-receive indexes the paths that CHANGED in a push. There is no such
20 * delta for a repo that just arrived, so this walks the tree at HEAD and
21 * indexes what is there, then delegates to the same indexChangedFiles() the
22 * hook uses — same filtering, same embedding model, same cap.
23 */
24
25import { mapWithConcurrency, DB_FANOUT_LIMIT } from "./concurrency";
26
27export interface IndexRepoResult {
28 indexed: number;
29 skipped: number;
30 model: string;
31 error?: string;
32}
33
34export interface IndexRepoDeps {
35 getTree: (
36 owner: string,
37 repo: string,
38 ref: string,
39 path?: string
40 ) => Promise<Array<{ name: string; type: string; path?: string }>>;
41 indexChangedFiles: (args: {
42 repositoryId: string;
43 ownerName: string;
44 repoName: string;
45 commitSha: string;
46 changedPaths: string[];
47 }) => Promise<{ indexed: number; skipped: number; model: string }>;
48}
49
50async function realDeps(): Promise<IndexRepoDeps> {
51 const { getTree } = await import("../git/repository");
52 const { indexChangedFiles } = await import("./semantic-index");
53 return { getTree: getTree as IndexRepoDeps["getTree"], indexChangedFiles };
54}
55
56/** Directories never worth embedding. Keeps the walk cheap on a big import. */
57const SKIP_DIRS = new Set([
58 ".git", "node_modules", "dist", "build", "out", "target", "vendor",
59 ".next", ".cache", "coverage", "__pycache__", ".venv", "venv",
60]);
61
62/** Depth cap. A pathological monorepo should not turn one import into a
63 * thousand tree reads; indexChangedFiles caps files independently. */
64const MAX_DEPTH = 6;
65
66/**
67 * Collect blob paths at `ref`, breadth-first, bounded.
68 *
69 * Concurrency is bounded for the same reason as everywhere else in the
70 * onboarding path: an import of 100 repos must not open an unbounded number
71 * of concurrent tree reads against one disk.
72 */
73async function collectPaths(
74 deps: IndexRepoDeps,
75 owner: string,
76 repo: string,
77 ref: string
78): Promise<string[]> {
79 const files: string[] = [];
80 let level: string[] = [""];
81
82 for (let depth = 0; depth < MAX_DEPTH && level.length > 0; depth++) {
83 const results = await mapWithConcurrency(level, DB_FANOUT_LIMIT, async (dir) => {
84 try {
85 return await deps.getTree(owner, repo, ref, dir || undefined);
86 } catch {
87 // A single unreadable directory must not abandon the whole index.
88 return [];
89 }
90 });
91
92 const next: string[] = [];
93 for (let i = 0; i < results.length; i++) {
94 const dir = level[i];
95 for (const entry of results[i] ?? []) {
96 const full = entry.path ?? (dir ? `${dir}/${entry.name}` : entry.name);
97 if (entry.type === "tree") {
98 if (SKIP_DIRS.has(entry.name)) continue;
99 next.push(full);
100 } else if (entry.type === "blob") {
101 files.push(full);
102 }
103 }
104 }
105 level = next;
106 }
107
108 return files;
109}
110
111/**
112 * Index a repository at `commitSha`. Never throws — an unindexed repo is a
113 * degraded search experience, not a failed import or a failed repo creation.
114 */
115export async function indexExistingRepo(
116 args: {
117 repositoryId: string;
118 owner: string;
119 repoName: string;
120 ref: string;
121 commitSha: string;
122 },
123 injected?: IndexRepoDeps
124): Promise<IndexRepoResult> {
125 try {
126 const deps = injected ?? (await realDeps());
127 const paths = await collectPaths(deps, args.owner, args.repoName, args.ref);
128 if (paths.length === 0) {
129 return { indexed: 0, skipped: 0, model: "none" };
130 }
131 // indexChangedFiles does the filtering (isCodeFile), the cap, and the
132 // embedding — reusing it keeps scaffolded, imported and pushed repos on
133 // exactly the same indexing behaviour.
134 return await deps.indexChangedFiles({
135 repositoryId: args.repositoryId,
136 ownerName: args.owner,
137 repoName: args.repoName,
138 commitSha: args.commitSha,
139 changedPaths: paths,
140 });
141 } catch (err) {
142 return {
143 indexed: 0,
144 skipped: 0,
145 model: "none",
146 error: err instanceof Error ? err.message : String(err),
147 };
148 }
149}
Modifiedsrc/lib/migration-onboarding.ts+30−0View fileUnifiedSplit
@@ -43,6 +43,8 @@ export interface MigrationRepoReport {
4343 secretsFound: number;
4444 /** Security findings (vulnerable patterns) in the imported code. */
4545 securityIssues: number;
46 /** Code files embedded into the semantic index. */
47 filesIndexed: number;
4648 /** Human-readable scan detail, already scrubbed by the scanner. */
4749 scanDetail: string;
4850 /** Set when this repo's onboarding failed; the others still ran. */
@@ -56,6 +58,8 @@ export interface MigrationReport {
5658 totalSecrets: number;
5759 totalSecurityIssues: number;
5860 reposBootstrapped: number;
61 /** Total code files embedded — what makes semantic search work at all. */
62 totalFilesIndexed: number;
5963 reposWithFindings: number;
6064}
6165
@@ -81,6 +85,7 @@ export async function runMigrationOnboarding(
8185 protectionCreated: false,
8286 secretsFound: 0,
8387 securityIssues: 0,
88 filesIndexed: 0,
8489 scanDetail: "",
8590 };
8691
@@ -124,6 +129,30 @@ export async function runMigrationOnboarding(
124129 base.error = base.error ? `${base.error}; ${msg}` : msg;
125130 }
126131
132 // 3. Semantic index. indexChangedFiles() is called only from the git
133 // post-receive hook, and a bulk import CLONES rather than pushes —
134 // so without this every imported repo had an empty index and
135 // semantic search, repo chat and "ask this repo" all returned
136 // nothing. Silently, because an empty index looks exactly like
137 // "no matches".
138 try {
139 const { indexExistingRepo } = await import("./index-existing-repo");
140 const idx = await indexExistingRepo({
141 repositoryId: repo.id,
142 owner: repo.owner,
143 repoName: repo.name,
144 ref: branch,
145 commitSha: branch,
146 });
147 base.filesIndexed = idx.indexed;
148 if (idx.error) {
149 base.error = base.error ? `${base.error}; index: ${idx.error}` : `index: ${idx.error}`;
150 }
151 } catch (err) {
152 const msg = `index: ${err instanceof Error ? err.message : String(err)}`;
153 base.error = base.error ? `${base.error}; ${msg}` : msg;
154 }
155
127156 return base;
128157 }
129158 );
@@ -133,6 +162,7 @@ export async function runMigrationOnboarding(
133162 totalRepos: results.length,
134163 totalSecrets: results.reduce((n, r) => n + r.secretsFound, 0),
135164 totalSecurityIssues: results.reduce((n, r) => n + r.securityIssues, 0),
165 totalFilesIndexed: results.reduce((n, r) => n + r.filesIndexed, 0),
136166 reposBootstrapped: results.filter((r) => r.bootstrapped).length,
137167 reposWithFindings: results.filter(
138168 (r) => r.secretsFound > 0 || r.securityIssues > 0
Modifiedsrc/routes/import-bulk.tsx+4−0View fileUnifiedSplit
@@ -1038,6 +1038,10 @@ importBulkRoutes.post("/import/bulk", requireAuth, async (c) => {
10381038 security finding
10391039 {migration.totalSecurityIssues === 1 ? "" : "s"}
10401040 </span>
1041 <span class="import-bulk-summary-stat">
1042 <span class="num">{migration.totalFilesIndexed}</span> files
1043 indexed for search
1044 </span>
10411045 </div>
10421046
10431047 {migration.reposWithFindings > 0 ? (
10441048