Skip to content

Commit e3d4be0

Browse files
committed
Merge remote-tracking branch 'origin/main' into feat/clawpatch-operating-loop
* origin/main: docs(changelog): note opencode diagnostics fix fix(provider): improve opencode JSON diagnostics fix(mapper): allow BOM before express imports fix(mapper): anchor express import scanning to declarations # Conflicts: # CHANGELOG.md
2 parents 4eb9a55 + 4954bbb commit e3d4be0

5 files changed

Lines changed: 107 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
- Fixed first-time `clawpatch open-pr` branch creation to start from the recorded patch base.
1212
- Fixed command execution so providers that exit before reading stdin do not surface benign `EPIPE` errors.
1313
- Fixed `clawpatch ci --since` empty-review output so it reports `reviewed: 0`.
14+
- Improved OpenCode malformed JSON diagnostics with output length, event kinds, and a bounded preview, thanks @rohitjavvadi.
1415
- Fixed Express route mapping for aliased Router imports that follow block comment banners, thanks @rohitjavvadi.
1516
- Fixed Bun package-manager detection to recognize the text `bun.lock` lockfile, thanks @austinm911.
1617

src/mapper.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1816,6 +1816,7 @@ describe("mapFeatures", () => {
18161816
"",
18171817
"const config = { import: true }",
18181818
"export type { Router as ExportedTypeRouter } from 'express';",
1819+
"const importPattern = /import { Router as RegexImportRouter } from 'express'/;",
18191820
"const app = express();",
18201821
"const otherRouter = OtherRouter();",
18211822
"const commentedOutRouter = CommentedOutRouter();",
@@ -1827,6 +1828,7 @@ describe("mapFeatures", () => {
18271828
"const fromBindingRouter = FromBindingRouter();",
18281829
"const commentedTypeRouter = CommentedTypeRouter();",
18291830
"const exportedTypeRouter = ExportedTypeRouter();",
1831+
"const regexImportRouter = RegexImportRouter();",
18301832
"const typedRouter: Router = Router();",
18311833
"const projectRouter = Router({ mergeParams: true });",
18321834
"let hitCount = 0;",
@@ -1847,6 +1849,7 @@ describe("mapFeatures", () => {
18471849
"fromBindingRouter.get('/from-binding-router', listFromBindingRouter);",
18481850
"commentedTypeRouter.get('/commented-type-router', ignoredCommentedTypeRouter);",
18491851
"exportedTypeRouter.get('/exported-type-router', ignoredExportedTypeRouter);",
1852+
"regexImportRouter.get('/regex-import-router', ignoredRegexImportRouter);",
18501853
"router.post<{ Body: CreateJob }>('/typed-jobs', createTypedJob);",
18511854
"typedRouter.patch('/typed/:id', updateTyped);",
18521855
"router.route('/users').get(listUsers).delete(deleteUsers);",
@@ -1874,6 +1877,7 @@ describe("mapFeatures", () => {
18741877
"function listFromBindingRouter() {}",
18751878
"function ignoredCommentedTypeRouter() {}",
18761879
"function ignoredExportedTypeRouter() {}",
1880+
"function ignoredRegexImportRouter() {}",
18771881
"function createTypedJob() {}",
18781882
"function updateTyped() {}",
18791883
"function listUsers() {}",
@@ -1975,6 +1979,18 @@ describe("mapFeatures", () => {
19751979
await writeFixture(root, "src/fastify.test.ts", "test('fastify', () => {});\n");
19761980
await writeFixture(root, "src/fastify-plugin.test.ts", "test('fastify plugin', () => {});\n");
19771981
await writeFixture(root, "src/hono.test.ts", "test('hono', () => {});\n");
1982+
await writeFixture(
1983+
root,
1984+
"src/bom-router.ts",
1985+
[
1986+
"\uFEFFimport { Router as BomRouter } from 'express';",
1987+
"",
1988+
"const bomRouter = BomRouter();",
1989+
"bomRouter.get('/bom-router', listBomRouter);",
1990+
"function listBomRouter() {}",
1991+
"",
1992+
].join("\n"),
1993+
);
19781994
await writeFixture(
19791995
root,
19801996
"src/mixed.tsx",
@@ -1983,8 +1999,12 @@ describe("mapFeatures", () => {
19831999
"",
19842000
"const app = express();",
19852001
"const view = <div></div>;",
2002+
"const docs = <code>import { Router as JsxImportRouter } from 'express'</code>;",
2003+
"const jsxImportRouter = JsxImportRouter();",
19862004
"app.get('/after-jsx-close', afterJsxClose);",
2005+
"jsxImportRouter.get('/jsx-import-router', ignoredJsxImportRouter);",
19872006
"function afterJsxClose() {}",
2007+
"function ignoredJsxImportRouter() {}",
19882008
"",
19892009
].join("\n"),
19902010
);
@@ -2061,6 +2081,7 @@ describe("mapFeatures", () => {
20612081
"Express route DELETE /users",
20622082
"Express route GET /reports",
20632083
"Express route GET /projects/:projectId/items",
2084+
"Express route GET /bom-router",
20642085
"Express route GET /after-jsx-close",
20652086
"Express route GET /custom-file-real",
20662087
"Fastify route GET /status",
@@ -2081,6 +2102,8 @@ describe("mapFeatures", () => {
20812102
expect(titles).not.toContain("Express route GET /commented-out-router");
20822103
expect(titles).not.toContain("Express route GET /commented-type-router");
20832104
expect(titles).not.toContain("Express route GET /exported-type-router");
2105+
expect(titles).not.toContain("Express route GET /regex-import-router");
2106+
expect(titles).not.toContain("Express route GET /jsx-import-router");
20842107
expect(titles).not.toContain("Express route GET /custom-import-router");
20852108
expect(titles).not.toContain("Express route GET /custom-router");
20862109
expect(titles).not.toContain("Express route GET /custom-alias-router");

src/mappers/node-routes.ts

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,10 @@ function expressRouterImportBindingNames(source: string): Set<string> {
339339
pattern.lastIndex = 0;
340340
for (const match of source.matchAll(pattern)) {
341341
const importIndex = match.index ?? 0;
342-
if (isInsideCommentOrString(source, importIndex)) {
342+
if (
343+
isInsideCommentOrString(source, importIndex) ||
344+
!isImportDeclarationStart(source, importIndex)
345+
) {
343346
continue;
344347
}
345348
const clause = readExpressStaticImportClause(source, importIndex);
@@ -553,6 +556,33 @@ function isImportClauseStart(char: string | undefined): boolean {
553556
return char !== undefined && (char === "{" || char === "*" || /[A-Za-z_$]/u.test(char));
554557
}
555558

559+
function isImportDeclarationStart(source: string, importIndex: number): boolean {
560+
let cursor = importIndex - 1;
561+
while (cursor >= 0) {
562+
const char = source[cursor];
563+
if (char === " " || char === "\t" || char === "\r" || char === "\uFEFF") {
564+
cursor -= 1;
565+
continue;
566+
}
567+
if (char === "\n") {
568+
return true;
569+
}
570+
if (char === "/" && source[cursor - 1] === "*") {
571+
const open = source.lastIndexOf("/*", cursor - 2);
572+
if (open < 0) {
573+
return false;
574+
}
575+
if (source.slice(open, cursor + 1).includes("\n")) {
576+
return true;
577+
}
578+
cursor = open - 1;
579+
continue;
580+
}
581+
return char === ";";
582+
}
583+
return true;
584+
}
585+
556586
function skipWhitespace(source: string, start: number): number {
557587
let cursor = start;
558588
while (/\s/u.test(source[cursor] ?? "")) {

src/provider.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { afterEach, describe, expect, it } from "vitest";
22
import { ClawpatchError } from "./errors.js";
33
import { __testing, extractJson, providerByName } from "./provider.js";
4+
import { safeProviderPreview } from "./provider-json.js";
45
import { revalidateOutputSchema, reviewOutputSchema } from "./types.js";
56

67
// eslint-disable-next-line no-underscore-dangle
@@ -55,6 +56,10 @@ function expectMalformed(fn: () => unknown, message: RegExp): void {
5556
throw new Error("expected malformed-output");
5657
}
5758

59+
function escapeRegExp(value: string): string {
60+
return value.replace(/[.*+?^${}()|[\]\\]/gu, "\\$&");
61+
}
62+
5863
describe("extractJson", () => {
5964
it("parses strict JSON directly", () => {
6065
const input = '{"findings":[],"inspected":{"files":[],"symbols":[],"notes":[]}}';
@@ -484,6 +489,46 @@ describe("extractOpencodeJson", () => {
484489
expectMalformed(() => extractOpencodeJson(stdout), /no extractable text.*step_finish/u);
485490
});
486491

492+
it("treats whitespace-only opencode text as no extractable text", () => {
493+
const stdout = [
494+
JSON.stringify({ type: "text", part: { text: " \n\t " } }),
495+
JSON.stringify({ type: "step_finish", part: { reason: "stop" } }),
496+
].join("\n");
497+
498+
expectMalformed(() => extractOpencodeJson(stdout), /no extractable text.*text, step_finish/u);
499+
});
500+
501+
it("throws malformed-output with a preview when opencode text is unparsable", () => {
502+
const stdout = [
503+
JSON.stringify({
504+
type: "text",
505+
part: { text: '{"findings": [' },
506+
}),
507+
JSON.stringify({ type: "step_finish", part: { reason: "stop" } }),
508+
].join("\n");
509+
510+
expectMalformed(
511+
() => extractOpencodeJson(stdout),
512+
/unparsable JSON.*text chars=14.*observed event kinds: \[text, step_finish\].*output preview: \{"findings": \[/u,
513+
);
514+
});
515+
516+
it("bounds the opencode unparsable text preview", () => {
517+
const text = `{"findings":["${"x".repeat(300)}`;
518+
const stdout = JSON.stringify({
519+
type: "text",
520+
part: { text },
521+
});
522+
const preview = safeProviderPreview(text);
523+
524+
expect(preview.length).toBe(200);
525+
526+
expectMalformed(
527+
() => extractOpencodeJson(stdout),
528+
new RegExp(`output preview: ${escapeRegExp(preview)}\\)`, "u"),
529+
);
530+
});
531+
487532
it("throws provider-failure for opencode error events", () => {
488533
const stdout = JSON.stringify({
489534
type: "error",

src/provider.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -700,7 +700,13 @@ export function extractOpencodeJson(stdout: string): unknown {
700700
}
701701
const parsed = extractJson(combined);
702702
if (parsed === null) {
703-
throw new ClawpatchError("opencode provider produced unparsable JSON", 8, "malformed-output");
703+
throw new ClawpatchError(
704+
`opencode provider produced unparsable JSON ` +
705+
`(text chars=${combined.length}, observed event kinds: ` +
706+
`[${[...observedKinds].join(", ")}], output preview: ${safeProviderPreview(combined)})`,
707+
8,
708+
"malformed-output",
709+
);
704710
}
705711
return parsed;
706712
}

0 commit comments

Comments
 (0)