Skip to content

Commit c2fbad4

Browse files
committed
fix(recall): reject repeated options and deduplicate diagnostics
1 parent 9555e93 commit c2fbad4

2 files changed

Lines changed: 29 additions & 2 deletions

File tree

‎scripts/history.test.mjs‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,12 +153,14 @@ test("missing stores and malformed records produce controlled diagnostics withou
153153
const options = { harness: "codex", workspace, root: join(root, "missing"), command: "list", env: {} };
154154
assert.deepEqual((await history(options)).sessions, []);
155155
jsonl(join(options.root, "sessions", "broken.jsonl"), ["PRIVATE INVALID HEADER"]);
156+
jsonl(join(options.root, "sessions", "also-broken.jsonl"), ["ANOTHER PRIVATE INVALID HEADER"]);
156157
const broken = await history(options);
157158
assert.equal(broken.warnings.length, 1);
158159
assert.doesNotMatch(JSON.stringify(broken), /PRIVATE/);
159160
jsonl(join(options.root, "sessions", "good.jsonl"), [{ type: "session_meta", payload: { id: "good", cwd: workspace } }, "PRIVATE INVALID BODY"]);
160161
const reading = await history({ ...options, command: "read", session: "good" });
161162
assert.deepEqual(reading.messages, []);
163+
assert.equal(reading.warnings.length, 2, "keep distinct diagnostics but emit each only once");
162164
assert.ok(reading.warnings.some((warning) => /malformed record/.test(warning)));
163165
assert.doesNotMatch(JSON.stringify(reading), /PRIVATE/);
164166
});
@@ -221,3 +223,25 @@ test("the recall CLI runs through a linked directory while imports stay silent",
221223
assert.equal(stdinImport.stdout, "");
222224
assert.equal(stdinImport.stderr, "");
223225
});
226+
227+
test("the recall CLI rejects duplicate single-value options and switches but permits repeated exclusions", (t) => {
228+
const { root, workspace } = fixture(t);
229+
const store = join(root, "codex");
230+
for (const id of ["first", "second"]) {
231+
jsonl(join(store, "sessions", `${id}.jsonl`), [{ type: "session_meta", payload: { id, cwd: workspace } }]);
232+
}
233+
const args = [resolve("skills/recall/scripts/history.mjs"), "list", "--harness", "codex", "--workspace", workspace, "--root", store];
234+
for (const [flag, suffix] of [
235+
["--workspace", ["--workspace", join(root, "other-workspace")]],
236+
["--limit", ["--limit", "1", "--limit", "2"]],
237+
["--local-text", ["--local-text", "--local-text"]],
238+
]) {
239+
const result = spawnSync(process.execPath, [...args, ...suffix], { encoding: "utf8", cwd: root });
240+
assert.notEqual(result.status, 0);
241+
assert.equal(result.stdout, "");
242+
assert.match(result.stderr, new RegExp(`Duplicate option: ${flag}`));
243+
}
244+
const excluded = spawnSync(process.execPath, [...args, "--exclude", "first", "--exclude", "second"], { encoding: "utf8", cwd: root });
245+
assert.equal(excluded.status, 0, excluded.stderr);
246+
assert.deepEqual(JSON.parse(excluded.stdout).sessions, []);
247+
});

‎skills/recall/scripts/history.mjs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ export async function history(input) {
235235
for (const session of candidates) if (!byId.has(session.id)) byId.set(session.id, session);
236236
const sessions = [...byId.values()];
237237
const publicSession = ({ id, workspace, updated }) => ({ id, workspace, updated: new Date(updated).toISOString() });
238-
if (options.command === "list") return { sessions: sessions.slice(0, options.limit).map(publicSession), truncated: sessions.length > options.limit, warnings };
238+
if (options.command === "list") return { sessions: sessions.slice(0, options.limit).map(publicSession), truncated: sessions.length > options.limit, warnings: [...new Set(warnings)] };
239239
const session = sessions.find((candidate) => candidate.id === options.session);
240240
if (!session) throw new Error("Session not found in the requested workspace, time window, or exclusions.");
241241
const messages = (await readMessages(session, options, warnings))
@@ -251,13 +251,16 @@ export async function history(input) {
251251
budget -= text.length;
252252
selected.push({ role: message.role, text });
253253
}
254-
return { session: publicSession(session), messages: selected.reverse(), sanitized: options.harness === "opencode" && !options.localText, truncated, warnings };
254+
return { session: publicSession(session), messages: selected.reverse(), sanitized: options.harness === "opencode" && !options.localText, truncated, warnings: [...new Set(warnings)] };
255255
}
256256

257257
function argumentsFor(argv) {
258258
const options = { command: argv[0], exclude: [] };
259259
const flags = { "--harness": "harness", "--workspace": "workspace", "--root": "root", "--since": "since", "--session": "session", "--leaf": "leaf", "--query": "query", "--limit": "limit", "--max-chars": "maxChars", "--exclude": "exclude" };
260+
const seen = new Set();
260261
for (let index = 1; index < argv.length; index++) {
262+
if (argv[index] !== "--exclude" && seen.has(argv[index])) throw new Error(`Duplicate option: ${argv[index]}`);
263+
seen.add(argv[index]);
261264
if (argv[index] === "--local-text") { options.localText = true; continue; }
262265
const key = flags[argv[index]];
263266
if (!key) throw new Error(`Unknown option: ${argv[index]}`);

0 commit comments

Comments
 (0)