Skip to content

Commit b7ad52e

Browse files
lewisnsmithclaude
andcommitted
Fix 16 code quality issues from comprehensive review
- Remove unused pino dep, dead spawnProxy function, unused variables - Dynamic CLI version from package.json, bump to 0.1.1 - Add closed guard to logger enqueue, flush pendingClientRequests on exit - Scope hallucination detection to tools/call only, add time window - Add SIGTERM handler to tailSession, incremental byte reads for tail - Shell-safe quoting in init commands, skip fake commands when no config - Add npx flight pattern detection for idempotent wrapping - Add tsconfig.test.json for test/bench type checking - Add close event to JsonRpcParser interface, fix test type errors - Fix alerts test to filter by session_id (avoid cross-test pollution) - Bump Node engine to >=20.11.0, CI matrix to 20.x/22.x - Add status note to PRD Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent edab297 commit b7ad52e

14 files changed

Lines changed: 103 additions & 74 deletions

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ jobs:
1111
runs-on: ubuntu-latest
1212
strategy:
1313
matrix:
14-
node-version: [20, 22]
14+
node-version: ['20.x', '22.x']
1515

1616
steps:
1717
- uses: actions/checkout@v4

docs/flight-prd.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# Product Requirements Document: Flight Proxy
22

3+
> **Status:** This document describes the full v1.0 vision. For current implementation status, see the [README](../README.md).
4+
35
## Overview
46

57
**Product Name:** Flight Proxy

package.json

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "flight-proxy",
3-
"version": "0.1.0",
3+
"version": "0.1.1",
44
"description": "MCP flight recorder and token optimizer for AI coding agents",
55
"type": "module",
66
"main": "dist/index.js",
@@ -14,7 +14,7 @@
1414
"test": "vitest run",
1515
"test:watch": "vitest",
1616
"lint": "eslint src/",
17-
"typecheck": "tsc --noEmit",
17+
"typecheck": "tsc --noEmit && tsc --project tsconfig.test.json",
1818
"bench": "npx tsx bench/throughput.ts",
1919
"check": "npm run lint && npm run typecheck && npm run test",
2020
"prepublishOnly": "npm run build"
@@ -30,17 +30,16 @@
3030
"tool-calling",
3131
"flight-recorder"
3232
],
33-
"author": "",
33+
"author": "Lewis Smith",
3434
"license": "MIT",
3535
"engines": {
36-
"node": ">=20"
36+
"node": ">=20.11.0"
3737
},
3838
"files": [
3939
"dist"
4040
],
4141
"dependencies": {
42-
"commander": "^14.0.3",
43-
"pino": "^10.3.1"
42+
"commander": "^14.0.3"
4443
},
4544
"devDependencies": {
4645
"@eslint/js": "^10.0.1",

src/cli.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
import { Command } from "commander";
2+
import { createRequire } from "node:module";
23
import { startProxy } from "./proxy.js";
34
import { initClaude, initClaudeCode, getClaudeConfigPath, getClaudeCodeConfigPath } from "./init.js";
45
import { listSessions, tailSession, viewSession, filterSessions, inspectCall, listAlerts } from "./log-commands.js";
56

7+
const require = createRequire(import.meta.url);
8+
const pkg = require("../package.json") as { version: string };
9+
610
const program = new Command();
711

812
program
913
.name("flight")
1014
.description("MCP flight recorder and token optimizer for AI coding agents")
11-
.version("0.2.0");
15+
.version(pkg.version);
1216

1317
program
1418
.command("proxy")

src/init.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,15 @@ export function wrapWithFlight(servers: Record<string, McpServerEntry>): Record<
3030
const wrapped: Record<string, McpServerEntry> = {};
3131

3232
for (const [name, server] of Object.entries(servers)) {
33-
// Skip if already wrapped
33+
// Skip if already wrapped (direct invocation or via npx)
3434
if (server.command === "flight" || server.command === "flight-proxy") {
3535
wrapped[name] = server;
3636
continue;
3737
}
38+
if (server.args?.[0] === "flight" && server.args?.[1] === "proxy") {
39+
wrapped[name] = server;
40+
continue;
41+
}
3842

3943
const args = ["proxy", "--cmd", server.command];
4044
if (server.args && server.args.length > 0) {
@@ -118,15 +122,17 @@ export async function initClaudeCode(options: ClaudeCodeInitOptions = {}): Promi
118122
};
119123
}
120124

121-
// Non-apply mode: print claude mcp add-json commands
125+
// Non-apply mode: print claude mcp add-json commands (only for real configs)
122126
const commands: string[] = [];
123-
for (const [name, server] of Object.entries(wrapped)) {
124-
const json = JSON.stringify(server);
125-
// Escape single quotes for shell safety: replace ' with '\''
126-
const escapedJson = json.replace(/'/g, "'\\''");
127-
const escapedName = name.replace(/'/g, "'\\''");
128-
const scopeFlag = scope === "project" ? " --scope project" : "";
129-
commands.push(`claude mcp add-json '${escapedName}' '${escapedJson}'${scopeFlag}`);
127+
if (configFound) {
128+
for (const [name, server] of Object.entries(wrapped)) {
129+
const json = JSON.stringify(server);
130+
// Escape single quotes for shell safety: replace ' with '\''
131+
const escapedJson = json.replace(/'/g, "'\\''");
132+
const escapedName = name.replace(/'/g, "'\\''");
133+
const scopeFlag = scope === "project" ? " --scope project" : "";
134+
commands.push(`claude mcp add-json '${escapedName}' '${escapedJson}'${scopeFlag}`);
135+
}
130136
}
131137

132138
// Also write snippet for reference

src/json-rpc.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export interface JsonRpcMessage {
1414
export interface JsonRpcParser extends EventEmitter {
1515
on(event: "message", listener: (msg: JsonRpcMessage) => void): this;
1616
on(event: "error", listener: (err: Error) => void): this;
17+
on(event: "close", listener: () => void): this;
1718
}
1819

1920
export function parseJsonRpcStream(input: Readable): JsonRpcParser {

src/log-commands.ts

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { readdir, readFile, stat } from "node:fs/promises";
1+
import { readdir, readFile, stat, open } from "node:fs/promises";
22
import { join } from "node:path";
33
import { homedir } from "node:os";
44
import { watch } from "node:fs";
@@ -163,36 +163,49 @@ export async function tailSession(sessionId?: string): Promise<void> {
163163
// ignore
164164
}
165165

166+
let partialLine = "";
167+
166168
const watcher = watch(filePath, async () => {
167169
try {
168170
const s = await stat(filePath);
169171
if (s.size <= lastSize) return;
170172

171-
const content = await readFile(filePath, "utf-8");
172-
const lines = content.trim().split("\n").filter(Boolean);
173-
174-
// Parse only genuinely new lines
175-
const newLines = lines.slice(lastLineCount);
176-
for (const line of newLines) {
177-
try {
178-
const entry = JSON.parse(line) as LogEntry;
179-
console.log(formatEntryLine(entry));
180-
} catch {
181-
// skip malformed lines during active writing
173+
// Read only new bytes from the file
174+
const fd = await open(filePath, "r");
175+
try {
176+
const newBytes = Buffer.alloc(s.size - lastSize);
177+
await fd.read(newBytes, 0, newBytes.length, lastSize);
178+
const chunk = partialLine + newBytes.toString("utf-8");
179+
const lines = chunk.split("\n");
180+
181+
// Last element may be a partial line (no trailing newline yet)
182+
partialLine = lines.pop() ?? "";
183+
184+
for (const line of lines) {
185+
if (!line.trim()) continue;
186+
try {
187+
const entry = JSON.parse(line) as LogEntry;
188+
console.log(formatEntryLine(entry));
189+
} catch {
190+
// skip malformed lines during active writing
191+
}
182192
}
193+
lastSize = s.size;
194+
} finally {
195+
await fd.close();
183196
}
184-
lastLineCount = lines.length;
185-
lastSize = s.size;
186197
} catch {
187198
// ignore read errors during active writing
188199
}
189200
});
190201

191-
// Keep alive until Ctrl+C
192-
process.once("SIGINT", () => {
202+
// Keep alive until terminated
203+
const cleanup = () => {
193204
watcher.close();
194205
process.exit(0);
195-
});
206+
};
207+
process.once("SIGINT", cleanup);
208+
process.once("SIGTERM", cleanup);
196209

197210
await new Promise(() => {}); // block forever
198211
}
@@ -234,7 +247,8 @@ export async function filterSessions(options: {
234247
let entries = await readLogEntries(file);
235248

236249
if (options.tool) {
237-
entries = entries.filter((e) => e.tool_name === options.tool || e.method.includes(options.tool!));
250+
const toolFilter = options.tool;
251+
entries = entries.filter((e) => e.tool_name === toolFilter || e.method.includes(toolFilter));
238252
}
239253
if (options.errors) {
240254
entries = entries.filter((e) => e.error);

src/logger.ts

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ export async function createSessionLogger(logDir?: string, redactionOptions?: Re
171171
}
172172

173173
function enqueue(line: string) {
174-
if (!loggingEnabled) return;
174+
if (!loggingEnabled || closed) return;
175175
if (logSizeCapped) return;
176176
if (writeQueue.length >= MAX_QUEUE_DEPTH) {
177177
process.stderr.write("[flight] Warning: log queue full, dropping entry\n");
@@ -203,17 +203,20 @@ export async function createSessionLogger(logDir?: string, redactionOptions?: Re
203203
let prevErrorMethod: string | undefined;
204204

205205
if (direction === "client->server") {
206-
// Hallucination detection: check if the most recent server response was an error
207-
// and this new request is for a different tool (not a retry)
208-
const lastResponse = recentResponses.length > 0
209-
? recentResponses[recentResponses.length - 1]
210-
: undefined;
211-
if (lastResponse?.isError && (now - lastResponse.timestamp) < HALLUCINATION_WINDOW_MS) {
212-
prevErrorToolName = lastResponse.toolName;
213-
prevErrorMethod = lastResponse.method;
214-
const isRetry = msg.method === lastResponse.method && toolName === lastResponse.toolName;
215-
if (!isRetry) {
216-
hallucinationHint = true;
206+
// Hallucination detection: only applies to tools/call requests following a tools/call error
207+
// (not notifications, resources/list, or other non-tool methods)
208+
if (msg.method === "tools/call") {
209+
const lastResponse = recentResponses.length > 0
210+
? recentResponses[recentResponses.length - 1]
211+
: undefined;
212+
if (lastResponse?.isError && lastResponse.method === "tools/call"
213+
&& (now - lastResponse.timestamp) < HALLUCINATION_WINDOW_MS) {
214+
prevErrorToolName = lastResponse.toolName;
215+
prevErrorMethod = lastResponse.method;
216+
const isRetry = toolName === lastResponse.toolName;
217+
if (!isRetry) {
218+
hallucinationHint = true;
219+
}
217220
}
218221
}
219222

src/proxy.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,11 +160,19 @@ export async function startProxy(options: ProxyOptions): Promise<void> {
160160

161161
// Handle upstream exit
162162
upstream.on("close", async (code) => {
163-
// Flush held error responses for any pending retries so the client isn't left hanging
163+
// Flush held error responses for any pending retries
164164
for (const heldError of pendingRetries.values()) {
165165
process.stdout.write(JSON.stringify(heldError) + "\n");
166166
}
167167
pendingRetries.clear();
168+
// Send error responses for any requests that never got a reply
169+
for (const [id] of pendingClientRequests) {
170+
process.stdout.write(JSON.stringify({
171+
jsonrpc: "2.0", id,
172+
error: { code: -32000, message: "Upstream process exited" },
173+
}) + "\n");
174+
}
175+
pendingClientRequests.clear();
168176
await logger.close();
169177
process.exit(code ?? 0);
170178
});

test/alerts.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,12 @@ describe("Alert System", () => {
126126

127127
const content = await readFile(alertPath, "utf-8");
128128
const lines = content.trim().split("\n").filter(Boolean);
129-
const lastAlert = JSON.parse(lines[lines.length - 1]) as AlertEntry;
129+
const sessionAlerts = lines
130+
.map((l) => JSON.parse(l) as AlertEntry)
131+
.filter((a) => a.session_id === logger.sessionId);
130132

133+
expect(sessionAlerts.length).toBeGreaterThanOrEqual(1);
134+
const lastAlert = sessionAlerts[sessionAlerts.length - 1];
131135
expect(lastAlert.severity).toBe("error");
132136
expect(lastAlert.message).toBe("Search failed");
133137
});

0 commit comments

Comments
 (0)