Skip to content

Commit 8e33d40

Browse files
eluce2claude
andcommitted
fix: address CodeRabbit review on PR 136
- encodeURIComponent table names in URL paths - validate --data is non-null object, --fields is array - structured dry-run output (no console.log prefix) - validate webhook IDs as decimal integers - read CLI version from package.json - handle primitive arrays in --pretty mode - use execFileSync in integration tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 2ca62b8 commit 8e33d40

7 files changed

Lines changed: 62 additions & 22 deletions

File tree

packages/fmodata/src/cli/commands/query.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ export function makeRecordsCommand(): Command {
6161
where: opts.where as string | undefined,
6262
orderBy: opts.orderBy as string | undefined,
6363
});
64-
const result = await db._makeRequest<{ value: unknown[] }>(`/${opts.table}${qs}`);
64+
const table = encodeURIComponent(opts.table as string);
65+
const result = await db._makeRequest<{ value: unknown[] }>(`/${table}${qs}`);
6566
if (result.error) {
6667
throw result.error;
6768
}
@@ -82,11 +83,16 @@ export function makeRecordsCommand(): Command {
8283
const { db } = buildConnection(globalOpts);
8384
let data: Record<string, unknown>;
8485
try {
85-
data = JSON.parse(opts.data) as Record<string, unknown>;
86+
const parsed: unknown = JSON.parse(opts.data);
87+
if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) {
88+
throw new Error();
89+
}
90+
data = parsed as Record<string, unknown>;
8691
} catch {
8792
throw new Error("--data must be a valid JSON object");
8893
}
89-
const result = await db._makeRequest<unknown>(`/${opts.table}`, {
94+
const table = encodeURIComponent(opts.table as string);
95+
const result = await db._makeRequest<unknown>(`/${table}`, {
9096
method: "POST",
9197
body: JSON.stringify(data),
9298
});
@@ -111,12 +117,17 @@ export function makeRecordsCommand(): Command {
111117
const { db } = buildConnection(globalOpts);
112118
let data: Record<string, unknown>;
113119
try {
114-
data = JSON.parse(opts.data) as Record<string, unknown>;
120+
const parsed: unknown = JSON.parse(opts.data);
121+
if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) {
122+
throw new Error();
123+
}
124+
data = parsed as Record<string, unknown>;
115125
} catch {
116126
throw new Error("--data must be a valid JSON object");
117127
}
118128
const qs = buildQueryString({ where: opts.where as string | undefined });
119-
const result = await db._makeRequest<unknown>(`/${opts.table}${qs}`, {
129+
const table = encodeURIComponent(opts.table as string);
130+
const result = await db._makeRequest<unknown>(`/${table}${qs}`, {
120131
method: "PATCH",
121132
body: JSON.stringify(data),
122133
});
@@ -139,7 +150,8 @@ export function makeRecordsCommand(): Command {
139150
try {
140151
const { db } = buildConnection(globalOpts);
141152
const qs = buildQueryString({ where: opts.where as string | undefined });
142-
const result = await db._makeRequest<unknown>(`/${opts.table}${qs}`, {
153+
const table = encodeURIComponent(opts.table as string);
154+
const result = await db._makeRequest<unknown>(`/${table}${qs}`, {
143155
method: "DELETE",
144156
});
145157
if (result.error) {

packages/fmodata/src/cli/commands/schema.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,15 @@ export function makeSchemaCommand(): Command {
3333
try {
3434
let fields: Field[];
3535
try {
36-
fields = JSON.parse(opts.fields) as Field[];
36+
const parsed: unknown = JSON.parse(opts.fields);
37+
if (!Array.isArray(parsed)) throw new Error();
38+
fields = parsed as Field[];
3739
} catch {
3840
throw new Error("--fields must be a valid JSON array");
3941
}
4042

4143
if (!opts.confirm) {
42-
console.log("[dry-run] Would create table:");
43-
printResult({ tableName: opts.name, fields }, { pretty: globalOpts.pretty ?? false });
44+
printResult({ dryRun: true, action: "create-table", tableName: opts.name, fields }, { pretty: globalOpts.pretty ?? false });
4445
return;
4546
}
4647

@@ -63,14 +64,15 @@ export function makeSchemaCommand(): Command {
6364
try {
6465
let fields: Field[];
6566
try {
66-
fields = JSON.parse(opts.fields) as Field[];
67+
const parsed: unknown = JSON.parse(opts.fields);
68+
if (!Array.isArray(parsed)) throw new Error();
69+
fields = parsed as Field[];
6770
} catch {
6871
throw new Error("--fields must be a valid JSON array");
6972
}
7073

7174
if (!opts.confirm) {
72-
console.log("[dry-run] Would add fields to table:");
73-
printResult({ tableName: opts.table, fields }, { pretty: globalOpts.pretty ?? false });
75+
printResult({ dryRun: true, action: "add-fields", tableName: opts.table, fields }, { pretty: globalOpts.pretty ?? false });
7476
return;
7577
}
7678

packages/fmodata/src/cli/commands/webhook.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@ import { buildConnection } from "../utils/connection";
44
import { handleCliError } from "../utils/errors";
55
import { printResult } from "../utils/output";
66

7+
function parseWebhookId(id: string): number {
8+
if (!/^\d+$/.test(id)) {
9+
throw new Error(`Invalid webhook ID: "${id}" — must be a positive integer`);
10+
}
11+
return Number(id);
12+
}
13+
714
export function makeWebhookCommand(): Command {
815
const webhook = new Command("webhook").description("FileMaker webhook operations");
916

@@ -27,8 +34,9 @@ export function makeWebhookCommand(): Command {
2734
.action(async (id: string, _opts, cmd) => {
2835
const globalOpts = cmd.parent?.parent?.opts() as ConnectionOptions & { pretty: boolean };
2936
try {
37+
const parsedId = parseWebhookId(id);
3038
const { db } = buildConnection(globalOpts);
31-
const result = await db.webhook.get(Number(id));
39+
const result = await db.webhook.get(parsedId);
3240
printResult(result, { pretty: globalOpts.pretty ?? false });
3341
} catch (err) {
3442
handleCliError(err);
@@ -96,9 +104,10 @@ export function makeWebhookCommand(): Command {
96104
.action(async (id: string, _opts, cmd) => {
97105
const globalOpts = cmd.parent?.parent?.opts() as ConnectionOptions & { pretty: boolean };
98106
try {
107+
const parsedId = parseWebhookId(id);
99108
const { db } = buildConnection(globalOpts);
100-
await db.webhook.remove(Number(id));
101-
printResult({ removed: true, id: Number(id) }, { pretty: globalOpts.pretty ?? false });
109+
await db.webhook.remove(parsedId);
110+
printResult({ removed: true, id: parsedId }, { pretty: globalOpts.pretty ?? false });
102111
} catch (err) {
103112
handleCliError(err);
104113
}

packages/fmodata/src/cli/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { createRequire } from "node:module";
12
import { Command } from "commander";
23
import { makeMetadataCommand } from "./commands/metadata";
34
import { makeRecordsCommand } from "./commands/query";
@@ -7,12 +8,15 @@ import { makeWebhookCommand } from "./commands/webhook";
78
import { ENV_NAMES } from "./utils/connection";
89
import { handleCliError } from "./utils/errors";
910

11+
const require = createRequire(import.meta.url);
12+
const { version } = require("../../package.json") as { version: string };
13+
1014
const program = new Command();
1115

1216
program
1317
.name("fmodata")
1418
.description("FileMaker OData CLI — query, script, webhook, metadata, and schema operations")
15-
.version("0.1.0")
19+
.version(version)
1620
.option("--server <url>", `FM server URL [env: ${ENV_NAMES.server}]`)
1721
.option("--database <name>", `FM database name [env: ${ENV_NAMES.db}]`)
1822
.option("--username <user>", `FM username [env: ${ENV_NAMES.username}]`)

packages/fmodata/src/cli/utils/output.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,16 @@ function printTable(data: unknown): void {
2424
return;
2525
}
2626

27+
// Array of primitives — render as single column
28+
if (Array.isArray(data) && data.length > 0) {
29+
const table = new Table({ head: ["Value"] });
30+
for (const value of data) {
31+
table.push([String(value ?? "")]);
32+
}
33+
console.log(table.toString());
34+
return;
35+
}
36+
2737
// Single object — render as key-value pairs
2838
if (typeof data === "object" && data !== null && !Array.isArray(data)) {
2939
const table = new Table({ head: ["Key", "Value"] });

packages/fmodata/tests/cli/integration/binary.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { execSync } from "node:child_process";
1+
import { execFileSync } from "node:child_process";
22
import { existsSync } from "node:fs";
33
import { resolve } from "node:path";
44
import { describe, expect, it } from "vitest";
@@ -21,7 +21,7 @@ describe("CLI binary integration", () => {
2121
console.warn("Skipping: CLI binary not built");
2222
return;
2323
}
24-
const output = execSync(`node ${CLI_PATH} --help`, { encoding: "utf8" });
24+
const output = execFileSync(process.execPath, [CLI_PATH, "--help"], { encoding: "utf8" });
2525
expect(output).toContain("fmodata");
2626
expect(output).toContain("Usage");
2727
});
@@ -31,7 +31,7 @@ describe("CLI binary integration", () => {
3131
console.warn("Skipping: CLI binary not built");
3232
return;
3333
}
34-
const output = execSync(`node ${CLI_PATH} records --help`, { encoding: "utf8" });
34+
const output = execFileSync(process.execPath, [CLI_PATH, "records", "--help"], { encoding: "utf8" });
3535
expect(output).toContain("list");
3636
expect(output).toContain("insert");
3737
expect(output).toContain("update");
@@ -43,7 +43,7 @@ describe("CLI binary integration", () => {
4343
console.warn("Skipping: CLI binary not built");
4444
return;
4545
}
46-
const output = execSync(`node ${CLI_PATH} metadata --help`, { encoding: "utf8" });
46+
const output = execFileSync(process.execPath, [CLI_PATH, "metadata", "--help"], { encoding: "utf8" });
4747
expect(output).toContain("get");
4848
expect(output).toContain("tables");
4949
});

packages/fmodata/tests/cli/unit/output.test.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,13 @@ describe("printResult", () => {
4141
expect(output).toContain("ok");
4242
});
4343

44-
it("falls back to JSON for non-object data in table mode", () => {
44+
it("prints single-column table for primitive arrays in table mode", () => {
4545
printResult(["a", "b", "c"], { pretty: true });
4646
const output = stdoutSpy.mock.calls[0]?.[0] as string;
47-
expect(output).toContain('"a"');
47+
expect(output).toContain("Value");
48+
expect(output).toContain("a");
49+
expect(output).toContain("b");
50+
expect(output).toContain("c");
4851
});
4952
});
5053

0 commit comments

Comments
 (0)