Skip to content

Commit 2232ec1

Browse files
committed
fix: remaining low-severity bugs from clawpatch review
1 parent 0c09d8e commit 2232ec1

19 files changed

Lines changed: 121 additions & 53 deletions

.changeset/fix-high-bugs-batch.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
"codemap": patch
33
---
44

5-
Fix high-severity bugs (describe.each parent stack, git porcelain -z paths, CLI symlink entry, pr-comment TTY stdin) and medium bugs (changed-since -z paths, perf baseline RUNS guard, qualified typeof, decorator args_text, for-of binding refs, HTTP body drain, benchmark regex validation, jsx INSERT RETURNING id, sqlite stmt cache on close).
5+
Fix high-severity bugs (describe.each parent stack, git porcelain -z paths, CLI symlink entry, pr-comment TTY stdin), medium bugs (changed-since -z paths, perf baseline RUNS guard, qualified typeof, decorator args_text, for-of binding refs, HTTP body drain, benchmark regex validation, jsx INSERT RETURNING id, sqlite stmt cache on close), and low bugs (CLI parse guards, incremental delete transaction, apply summary.files, extension case, pnpm # paths, coverage db.transaction, impact walk delimiter, worker errors, pointer dedup, benchmark readAll visibility).

src/adapters/builtin.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { BUILTIN_ADAPTERS, getAdapterForExtension } from "./builtin";
55
describe("getAdapterForExtension", () => {
66
it("resolves TS/JS extensions to builtin.ts-js", () => {
77
expect(getAdapterForExtension(".ts")?.id).toBe("builtin.ts-js");
8+
expect(getAdapterForExtension(".TS")?.id).toBe("builtin.ts-js");
89
expect(getAdapterForExtension(".tsx")?.id).toBe("builtin.ts-js");
910
});
1011

src/adapters/builtin.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@ function buildAdapterIndex(
109109
// First-match-wins: skip ext if an earlier adapter already claimed it.
110110
for (const a of adapters) {
111111
for (const ext of a.extensions) {
112-
if (!index.has(ext)) index.set(ext, a);
112+
const key = ext.toLowerCase();
113+
if (!index.has(key)) index.set(key, a);
113114
}
114115
}
115116
return index;
@@ -129,5 +130,5 @@ export function getAdapterForExtension(
129130
index = buildAdapterIndex(adapters);
130131
adapterIndexCache.set(adapters, index);
131132
}
132-
return index.get(ext);
133+
return index.get(ext.toLowerCase());
133134
}

src/agents-init.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,31 @@ describe("upsertCodemapPointerFile", () => {
369369
}
370370
});
371371

372+
it("replaces ALL managed sections when file has two blocks", () => {
373+
const dir = mkdtempSync(join(tmpdir(), "codemap-pointer-"));
374+
const p = join(dir, "NOTE.md");
375+
try {
376+
writeFileSync(
377+
p,
378+
`${wrapPointerTest("FIRST")}\nOther content\n${wrapPointerTest("SECOND")}`,
379+
"utf-8",
380+
);
381+
upsertCodemapPointerFile(
382+
p,
383+
"UPDATED\n\nstill https://github.com/stainless-code/codemap\n`.agents/skills/codemap`\n`codemap query`",
384+
"NOTE.md",
385+
false,
386+
);
387+
const out = readFileSync(p, "utf-8");
388+
expect(out).toContain("UPDATED");
389+
expect(out).not.toContain("FIRST");
390+
expect(out).not.toContain("SECOND");
391+
expect(out.match(new RegExp(CODMAP_POINTER_BEGIN, "g"))?.length).toBe(1);
392+
} finally {
393+
rmSync(dir, { recursive: true, force: true });
394+
}
395+
});
396+
372397
it("--force replaces entire file with managed section", () => {
373398
const dir = mkdtempSync(join(tmpdir(), "codemap-pointer-"));
374399
const p = join(dir, "AGENTS.md");

src/agents-init.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -197,10 +197,10 @@ function escapeRegexChars(s: string): string {
197197
return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
198198
}
199199

200-
function codemapPointerBlockRegex(): RegExp {
200+
function codemapPointerBlockRegex(flags = "m"): RegExp {
201201
return new RegExp(
202202
`${escapeRegexChars(CODMAP_POINTER_BEGIN)}\\s*[\\s\\S]*?${escapeRegexChars(CODMAP_POINTER_END)}`,
203-
"m",
203+
flags,
204204
);
205205
}
206206

@@ -247,10 +247,15 @@ export function upsertCodemapPointerFile(
247247
}
248248

249249
const content = readFileSync(path, "utf-8");
250-
const re = codemapPointerBlockRegex();
251250

252-
if (content.match(re)) {
253-
const next = content.replace(re, wrapped);
251+
if (content.match(codemapPointerBlockRegex())) {
252+
const stripped = content
253+
.replace(codemapPointerBlockRegex("gm"), "")
254+
.replace(/\n{3,}/g, "\n\n")
255+
.trimEnd();
256+
const sep = stripped.length === 0 || stripped.endsWith("\n") ? "" : "\n\n";
257+
const next =
258+
stripped.length === 0 ? wrapped : `${stripped}${sep}${wrapped}`;
254259
if (next === content) {
255260
console.log(` Codemap section in ${label} already up to date`);
256261
return;

src/application/apply-engine.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -753,6 +753,7 @@ describe("applyDiffPayload", () => {
753753
});
754754

755755
expect(result.summary.rows).toBe(0); // row silently dropped
756+
expect(result.summary.files).toBe(0);
756757
expect(result.files).toEqual([]);
757758
});
758759
});

src/application/apply-engine.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ export function applyDiffPayload(opts: ApplyDiffPayloadOpts): ApplyJsonPayload {
133133
// would split phase 2 mid-loop and leak Q2 (c) all-or-nothing.
134134
const seenLines = new Map<string, Set<number>>();
135135
let validRows = 0;
136+
const distinctInputFiles = new Set<string>();
136137

137138
for (const row of rows) {
138139
const filePath = readString(row, "file_path");
@@ -151,6 +152,11 @@ export function applyDiffPayload(opts: ApplyDiffPayloadOpts): ApplyJsonPayload {
151152
continue;
152153
}
153154
validRows++;
155+
distinctInputFiles.add(
156+
isAbsolute(filePath) || !isWithinProjectRoot(resolvedRoot, filePath)
157+
? filePath
158+
: canonicalizeProjectFilePath(resolvedRoot, filePath),
159+
);
154160

155161
// Path-containment guard — without it `file_path: "../escape.ts"` would
156162
// write sibling-of-root files (CLI + MCP + HTTP all share this engine).
@@ -261,20 +267,6 @@ export function applyDiffPayload(opts: ApplyDiffPayloadOpts): ApplyJsonPayload {
261267
}
262268

263269
const filesWithConflicts = new Set(conflicts.map((c) => c.file_path)).size;
264-
const distinctInputFiles = new Set<string>();
265-
for (const row of rows) {
266-
const filePath = readString(row, "file_path");
267-
if (filePath === undefined) continue;
268-
// Count distinct disk targets, not distinct spellings — same as the
269-
// dedup applied to the cache + pending keys.
270-
if (isAbsolute(filePath) || !isWithinProjectRoot(resolvedRoot, filePath)) {
271-
distinctInputFiles.add(filePath);
272-
} else {
273-
distinctInputFiles.add(
274-
canonicalizeProjectFilePath(resolvedRoot, filePath),
275-
);
276-
}
277-
}
278270

279271
// Q2 (c) — any conflict aborts the run; dry-run never writes. Same Q5 envelope.
280272
if (dryRun || conflicts.length > 0) {

src/application/coverage-engine.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,7 @@ export function upsertCoverageRows(opts: UpsertOpts): IngestResult {
144144
// we're ingesting (a re-ingest is a full replace per file, not a merge),
145145
// then bulk-insert the new aggregates. Idempotent across re-runs.
146146
let pruned = 0;
147-
db.run("BEGIN");
148-
try {
147+
const persist = db.transaction(() => {
149148
for (const file_path of filesSeen) {
150149
db.run("DELETE FROM coverage WHERE file_path = ?", [file_path]);
151150
}
@@ -203,12 +202,8 @@ export function upsertCoverageRows(opts: UpsertOpts): IngestResult {
203202
"coverage_last_ingested_format",
204203
format,
205204
]);
206-
207-
db.run("COMMIT");
208-
} catch (err) {
209-
db.run("ROLLBACK");
210-
throw err;
211-
}
205+
});
206+
persist();
212207

213208
return {
214209
ingested: { symbols: buckets.size, files: filesSeen.size },

src/application/impact-engine.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -290,14 +290,14 @@ function walkCalls(db: CodemapDatabase, opts: WalkOpts): ImpactNode[] {
290290
// Seed depth = 0; `WHERE depth > 0` filters seed; `< depthLimit` is the cap.
291291
const sql = `
292292
WITH RECURSIVE walk(node, depth, path, file_path) AS (
293-
SELECT ?, 0, ',' || ? || ',', NULL
293+
SELECT ?, 0, char(30) || ? || char(30), NULL
294294
UNION ALL
295295
SELECT c.${joinToCol}, walk.depth + 1,
296-
walk.path || c.${joinToCol} || ',', c.file_path
296+
walk.path || c.${joinToCol} || char(30), c.file_path
297297
FROM calls c
298298
JOIN walk ON c.${joinFromCol} = walk.node
299299
WHERE walk.depth < ?
300-
AND instr(walk.path, ',' || c.${joinToCol} || ',') = 0
300+
AND instr(walk.path, char(30) || c.${joinToCol} || char(30)) = 0
301301
)
302302
SELECT node, depth, file_path
303303
FROM (
@@ -359,14 +359,14 @@ function walkFileGraph(db: CodemapDatabase, opts: WalkOpts): ImpactNode[] {
359359

360360
const sql = `
361361
WITH RECURSIVE walk(node, depth, path) AS (
362-
SELECT ?, 0, ',' || ? || ','
362+
SELECT ?, 0, char(30) || ? || char(30)
363363
UNION ALL
364364
SELECT c.${joinToCol}, walk.depth + 1,
365-
walk.path || c.${joinToCol} || ','
365+
walk.path || c.${joinToCol} || char(30)
366366
FROM ${table} c
367367
JOIN walk ON c.${joinFromCol} = walk.node
368368
WHERE walk.depth < ?
369-
AND instr(walk.path, ',' || c.${joinToCol} || ',') = 0
369+
AND instr(walk.path, char(30) || c.${joinToCol} || char(30)) = 0
370370
${filterNonNull}
371371
)
372372
SELECT node, MIN(depth) AS depth

src/application/index-engine.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,11 +91,11 @@ const TS_EXTENSIONS = new Set([
9191
const CSS_EXTENSIONS = new Set([".css"]);
9292

9393
function langFromExt(ext: string): string {
94-
return LANG_MAP[ext] ?? "text";
94+
return LANG_MAP[ext.toLowerCase()] ?? "text";
9595
}
9696

9797
function fileCategory(path: string): "ts" | "css" | "text" {
98-
const ext = extname(path);
98+
const ext = extname(path).toLowerCase();
9999
if (TS_EXTENSIONS.has(ext)) return "ts";
100100
if (CSS_EXTENSIONS.has(ext)) return "css";
101101
return "text";
@@ -424,6 +424,8 @@ export async function indexFiles(
424424
sourceCache?: ChangedSourceCache;
425425
/** When set, incremental branch skips its own `getAllFileHashes(db)` call. */
426426
existingHashes?: Map<string, string>;
427+
/** When set, incremental branch deletes these paths inside the same transaction as re-indexing. */
428+
deletedPaths?: string[];
427429
},
428430
): Promise<IndexRunStats> {
429431
const quiet = options?.quiet ?? false;
@@ -485,6 +487,10 @@ export async function indexFiles(
485487
const sourceCache = options?.sourceCache;
486488

487489
const transaction = db.transaction(() => {
490+
const deleted = options?.deletedPaths ?? [];
491+
if (deleted.length > 0) {
492+
deleteFilesFromIndex(db, deleted, quiet);
493+
}
488494
for (const relPath of filePaths) {
489495
const absPath = join(root, relPath);
490496
let source: string;

0 commit comments

Comments
 (0)