Skip to content

Commit b5192ec

Browse files
committed
fix: F1 fallback - add inferProviderFromBaseURL for bare model names
- Add inferProviderFromBaseURL() helper using URL API (hostname.endsWith) - Add short-circuit guard to prevent baseURL from overriding qualified model refs - Fix void 0 to undefined for consistency Addresses F1 feedback from PR #618 review
1 parent 9193ba6 commit b5192ec

4 files changed

Lines changed: 352 additions & 16 deletions

File tree

index.ts

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,29 @@ function splitProviderModel(modelRef: string): { provider?: string; model?: stri
702702
return { model: s };
703703
}
704704

705+
/**
706+
* 當 modelRef 是 bare name(無 / 前綴)時,從 baseURL 推斷 provider。
707+
* 使用 hostname.endsWith() 避免 fake-minimax.io 這類攻擊
708+
*/
709+
function inferProviderFromBaseURL(baseURL: string | undefined): string | undefined {
710+
if (!baseURL) return undefined;
711+
712+
try {
713+
const url = new URL(baseURL);
714+
const hostname = url.hostname.toLowerCase();
715+
716+
// 使用 endsWith 避免子網域欺騙
717+
if (hostname.endsWith("minimax.io")) return "minimax-portal";
718+
if (hostname.endsWith("openai.com")) return "openai";
719+
if (hostname.endsWith("anthropic.com")) return "anthropic";
720+
721+
return undefined;
722+
} catch {
723+
// 無效 URL 回傳 undefined
724+
return undefined;
725+
}
726+
}
727+
705728
function asNonEmptyString(value: unknown): string | undefined {
706729
if (typeof value !== "string") return undefined;
707730
const trimmed = value.trim();
@@ -1158,9 +1181,14 @@ async function generateReflectionText(params: {
11581181
onLog: onRetryLog,
11591182
execute: async () => {
11601183
const runEmbeddedPiAgent = await loadEmbeddedPiRunner();
1161-
const modelRef = resolveAgentPrimaryModelRef(params.cfg, params.agentId)
1162-
?? (((params.cfg as Record<string, unknown>)?.llm as Record<string, unknown>)?.model as string | undefined);
1163-
const { provider, model } = modelRef ? splitProviderModel(modelRef) : { provider: void 0, model: void 0 };
1184+
const cfg = params.cfg as Record<string, unknown>;
1185+
const llmConfig = cfg?.llm as Record<string, unknown> | undefined;
1186+
const modelRef =
1187+
(resolveAgentPrimaryModelRef(params.cfg, params.agentId) as string | undefined)
1188+
?? (llmConfig?.model as string | undefined);
1189+
const split = modelRef ? splitProviderModel(modelRef) : { provider: undefined, model: undefined };
1190+
const provider = split.provider ?? inferProviderFromBaseURL(llmConfig?.baseURL as string | undefined);
1191+
const model = split.model;
11641192
const embeddedTimeoutMs = Math.max(params.timeoutMs + 5000, 15000);
11651193

11661194
return await withTimeout(

src/store.ts

Lines changed: 71 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ async function loadLockfile(): Promise<any> {
6464
return lockfileModule;
6565
}
6666

67+
/** For unit testing: override the lockfile module with a mock. */
68+
export function __setLockfileModuleForTests(module: any): void {
69+
lockfileModule = module;
70+
}
71+
6772
export const loadLanceDB = async (): Promise<
6873
typeof import("@lancedb/lancedb")
6974
> => {
@@ -209,11 +214,70 @@ export class MemoryStore {
209214
try { mkdirSync(dirname(lockPath), { recursive: true }); } catch {}
210215
try { const { writeFileSync } = await import("node:fs"); writeFileSync(lockPath, "", { flag: "wx" }); } catch {}
211216
}
217+
// 【修復 #415】調整 retries:max wait 從 ~3100ms → ~151秒
218+
// 指數退避:1s, 2s, 4s, 8s, 16s, 30s×5,總計約 151 秒
219+
// ECOMPROMISED 透過 onCompromised callback 觸發(非 throw),使用 flag 機制正確處理
220+
let isCompromised = false;
221+
let compromisedErr: unknown = null;
222+
let fnSucceeded = false;
223+
let fnError: unknown = null;
224+
212225
const release = await lockfile.lock(lockPath, {
213-
retries: { retries: 5, factor: 2, minTimeout: 100, maxTimeout: 2000 },
214-
stale: 10000,
226+
retries: {
227+
retries: 10, // 從 5 → 10,讓 max wait 可覆蓋更長的 event loop block
228+
factor: 2,
229+
minTimeout: 1000, // 從 100 → 1000,避免過度密集重試打爆系統
230+
maxTimeout: 30000, // 從 2000 → 30000,讓單次重試等待夠長
231+
},
232+
stale: 10000, // 10 秒後視為 stale,觸發 ECOMPROMISED callback
233+
// 注意:ECOMPROMISED 是 ambiguous degradation 訊號,mtime 無法區分
234+
// "holder 崩潰" vs "holder event loop 阻塞",所以不嘗試區分
235+
onCompromised: (err: unknown) => {
236+
// 【修復 #415 關鍵】必須是同步 callback
237+
// setLockAsCompromised() 不等待 Promise,async throw 無法傳回 caller
238+
isCompromised = true;
239+
compromisedErr = err;
240+
},
215241
});
216-
try { return await fn(); } finally { await release(); }
242+
243+
try {
244+
const result = await fn();
245+
fnSucceeded = true;
246+
return result;
247+
} catch (e: unknown) {
248+
fnError = e;
249+
throw e;
250+
} finally {
251+
if (isCompromised) {
252+
// fnError 優先:fn() 失敗時,fn 的錯誤比 compromised 重要
253+
if (fnError !== null) {
254+
throw fnError;
255+
}
256+
// fn() 尚未完成就 compromised → throw,讓 caller 知道要重試
257+
if (!fnSucceeded) {
258+
throw compromisedErr as Error;
259+
}
260+
// fn() 成功執行,但 lock 在執行期間被標記 compromised
261+
// 正確行為:回傳成功結果(資料已寫入),明確告知 caller 不要重試
262+
console.warn(
263+
`[memory-lancedb-pro] Returning successful result despite compromised lock at "${lockPath}". ` +
264+
`Callers must not retry this operation automatically.`,
265+
);
266+
// 【修復 #415】compromised 後 release() 會回 ERELEASED,忽略即可
267+
// 重要:不要在這裡 return!否則 finally 的 return 會覆蓋 try 的 return 值
268+
try {
269+
await release();
270+
} catch (e: unknown) {
271+
if ((e as NodeJS.ErrnoException).code === 'ERELEASED') {
272+
// ERELEASED 是預期行為,不做任何事,讓 try 的 return 值通過
273+
} else {
274+
throw e; // 其他錯誤照拋
275+
}
276+
}
277+
} else {
278+
await release();
279+
}
280+
}
217281
}
218282

219283
get dbPath(): string {
@@ -387,9 +451,10 @@ export class MemoryStore {
387451
return this.runWithFileLock(async () => {
388452
try {
389453
await this.table!.add([fullEntry]);
390-
} catch (err: any) {
391-
const code = err.code || "";
392-
const message = err.message || String(err);
454+
} catch (err: unknown) {
455+
const e = err as { code?: string; message?: string };
456+
const code = e.code || "";
457+
const message = e.message || String(err);
393458
throw new Error(
394459
`Failed to store memory in "${this.config.dbPath}": ${code} ${message}`,
395460
);
@@ -444,12 +509,6 @@ export class MemoryStore {
444509
return res.length > 0;
445510
}
446511

447-
/** Lightweight total row count via LanceDB countRows(). */
448-
async count(): Promise<number> {
449-
await this.ensureInitialized();
450-
return await this.table!.countRows();
451-
}
452-
453512
async getById(id: string, scopeFilter?: string[]): Promise<MemoryEntry | null> {
454513
await this.ensureInitialized();
455514

test/cross-process-lock.test.mjs

Lines changed: 114 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { join } from "node:path";
66
import jitiFactory from "jiti";
77

88
const jiti = jitiFactory(import.meta.url, { interopDefault: true });
9-
const { MemoryStore } = jiti("../src/store.ts");
9+
const { MemoryStore, __setLockfileModuleForTests } = jiti("../src/store.ts");
1010

1111
function makeStore() {
1212
const dir = mkdtempSync(join(tmpdir(), "memory-lancedb-pro-lock-"));
@@ -26,6 +26,46 @@ function makeEntry(i) {
2626
}
2727

2828
describe("Cross-process file lock", () => {
29+
it("returns fn() result when onCompromised fires after fn() succeeds", async () => {
30+
// Regression test: fn() succeeded → isCompromised=true → finally must NOT throw
31+
// P1 bug: finally { return; } overrides try return with undefined (wrong)
32+
// Fix: fnSucceeded flag so finally knows whether fn() completed
33+
// Bug 2 fix: release() returns ERELEASED after compromised, finally must handle it
34+
const { store, dir } = makeStore();
35+
let compromiseCallback;
36+
let lockReleased = false; // 模擬 proper-lockfile 的 lock.released 行為
37+
38+
__setLockfileModuleForTests({
39+
lock: async (_lockPath, options) => {
40+
compromiseCallback = options.onCompromised;
41+
// 模擬 proper-lockfile: onCompromised 會設定 lock.released=true
42+
// release() 被呼叫時,若 lockReleased=true,回 ERELEASED
43+
return async () => {
44+
if (lockReleased) {
45+
const e = Object.assign(new Error("Lock is already released"), { code: "ERELEASED" });
46+
throw e;
47+
}
48+
};
49+
},
50+
});
51+
52+
try {
53+
const result = await store.runWithFileLock(async () => {
54+
// Simulate stale timeout firing DURING fn() execution
55+
// 這會觸發 onCompromised,設定 lockReleased=true
56+
compromiseCallback?.(Object.assign(new Error("stale lock"), { code: "ECOMPROMISED" }));
57+
lockReleased = true; // 模擬 setLockAsCompromised 設定 lock.released=true
58+
return "success-value";
59+
});
60+
61+
// MUST return the actual result, NOT undefined
62+
assert.strictEqual(result, "success-value");
63+
} finally {
64+
__setLockfileModuleForTests(null);
65+
rmSync(dir, { recursive: true, force: true });
66+
}
67+
});
68+
2969
it("creates .memory-write.lock file on first write", async () => {
3070
const { store, dir } = makeStore();
3171
try {
@@ -116,4 +156,77 @@ describe("Cross-process file lock", () => {
116156
rmSync(dir, { recursive: true, force: true });
117157
}
118158
});
159+
160+
it("fn() error takes priority over compromised error", async () => {
161+
// fnError 優先於 compromisedErr:fn() 失敗時,fn 的錯誤比 compromised 重要
162+
const { store, dir } = makeStore();
163+
let compromiseCallback;
164+
let lockReleased = false;
165+
166+
__setLockfileModuleForTests({
167+
lock: async (_lockPath, options) => {
168+
compromiseCallback = options.onCompromised;
169+
return async () => {
170+
if (lockReleased) {
171+
throw Object.assign(new Error("Lock is already released"), { code: "ERELEASED" });
172+
}
173+
};
174+
},
175+
});
176+
177+
try {
178+
await assert.rejects(
179+
async () => {
180+
await store.runWithFileLock(async () => {
181+
// compromised fires first, then fn() throws
182+
compromiseCallback?.(Object.assign(new Error("stale lock"), { code: "ECOMPROMISED" }));
183+
lockReleased = true;
184+
throw Object.assign(new Error("fn error"), { code: "EFNERROR" });
185+
});
186+
},
187+
(e) => {
188+
// 應該 throw fnError,不是 compromisedErr 或 ERELEASED
189+
assert.strictEqual(e.code, "EFNERROR", "should throw fnError, not compromisedErr");
190+
return true;
191+
},
192+
);
193+
} finally {
194+
__setLockfileModuleForTests(null);
195+
rmSync(dir, { recursive: true, force: true });
196+
}
197+
});
198+
199+
// 備註:以下情境無法有效測試
200+
// "fn() 尚未完成就 compromised":當 fn() 真的 pending 不結束時,
201+
// finally 不會執行(finally 只在 fn() resolve/reject 後才執行)。
202+
// 這是 JS Promise 語義,無法從外部打斷。實際行為:caller 會跟著一起 hang。
203+
// 此情境的正確處理方式是 caller 自行實作 timeout,runWithFileLock 層面無法處理。
204+
205+
it("non-compromised release() error propagates", async () => {
206+
// 非 compromised 時 release() 失敗,應往外拋
207+
const { store, dir } = makeStore();
208+
209+
__setLockfileModuleForTests({
210+
lock: async (_lockPath, _options) => {
211+
return async () => {
212+
throw Object.assign(new Error("release failed"), { code: "ERLSEFAIL" });
213+
};
214+
},
215+
});
216+
217+
try {
218+
await assert.rejects(
219+
async () => {
220+
await store.runWithFileLock(async () => "success");
221+
},
222+
(e) => {
223+
assert.strictEqual(e.code, "ERLSEFAIL", "should propagate release error");
224+
return true;
225+
},
226+
);
227+
} finally {
228+
__setLockfileModuleForTests(null);
229+
rmSync(dir, { recursive: true, force: true });
230+
}
231+
});
119232
});

0 commit comments

Comments
 (0)