diff --git a/CHANGELOG.md b/CHANGELOG.md index 21fcfbd..4daa9cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,18 @@ All notable changes to this project will be documented in this file. ## Unreleased -## 0.5.23 - 2026-08-02 +## 0.5.24 - 2026-08-05 + +### Added +- **`--sender` is the unambiguous spelling of the sender filter on `read`, `search`, and `export`.** `--from` keeps working and keeps its exact meaning on those verbs, so no existing caller's result set changes; the two disagreeing is a hard error rather than a silent precedence rule. + +### Fixed +- **A sender filter on `read`, `search`, or `export` can no longer produce a silent false absence.** `--from` names the CALLER on 26 subcommands and filters on `from_agent` on those three, so the canonical liveness probe `search --channel --from ` appended `AND from_agent = ` and became unsatisfiable by construction — a dispatched sub-agent is a different sender, so the one message being looked for is the one the filter removed. It answered `No messages found.` at rc=0 with an empty stderr. `--from` now always announces on stderr that it was applied as a sender filter, and an empty result from any of the three names the filters that produced it, so a zero caused by the caller's own query is distinguishable from a genuinely empty store (#807d355d, #e60b8820). +- **A blank `--sender` / `--from` is refused instead of silently dropping the filter.** `--sender "$WHO"` with `WHO` unset would otherwise return every sender's messages at exit code 0 and read as one sender's — the wrong-full direction, which is acted on rather than noticed. + +### Known gaps +- The disclosure covers the sender/recipient/channel/session/since dimensions. `--limit`, `--cursor`, and `--unread` are **not** yet named in it, so `read --cursor 999` against a populated channel is still a bare zero, and `read --channel X --cursor 999` prints an applied-filter line that omits the cursor. Tracked separately. +- The MCP surface (`src/mcp/tools/messaging.ts`) is unchanged and still carries the original ambiguity, including `read_messages` using `from` as caller identity and sender filter in the same call. Tracked separately. ### Added - **`conversations watch` can opt into full redacted channel content and monitor several identities in one process.** `--full-content` preserves actionable identifiers that the compact preview strips, while comma-separated `--from` values union independent inboxes without changing which identity owns writes (#74). diff --git a/package.json b/package.json index 1cc3a7e..1838dd0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@hasna/conversations", - "version": "0.5.23", + "version": "0.5.24", "description": "Real-time CLI messaging for AI agents", "type": "module", "bin": { diff --git a/src/cli/commands/messaging.ts b/src/cli/commands/messaging.ts index e8e609f..5472fbc 100644 --- a/src/cli/commands/messaging.ts +++ b/src/cli/commands/messaging.ts @@ -21,6 +21,13 @@ import type { DigestResult } from "../../lib/messages.js"; import { printErrorLine, printJson, printJsonLine, printLine } from "../../lib/stdout.js"; import { normalizeChannelName } from "../../lib/channel-names.js"; import { parseMessageReference } from "../../lib/message-reference.js"; +import { + discloseEmptyResult, + FROM_ALIAS_HELP, + noteSenderFilterAlias, + resolveSenderFilter, + SENDER_HELP, +} from "../sender-filter.js"; function quoteDigestCommandArg(value: string): string { return /^[A-Za-z0-9._:/@=-]+$/.test(value) ? value : `'${value.replace(/'/g, "'\\''")}'`; @@ -126,7 +133,8 @@ export function registerMessagingCommands(program: Command): void { .command("read") .description("Read messages") .option("--session ", "Filter by session ID") - .option("--from ", "Filter by sender") + .option("--sender ", SENDER_HELP) + .option("--from ", FROM_ALIAS_HELP) .option("--to ", "Filter by recipient") .option("--channel ", "Filter by channel") .option("--since ", "Messages after this ISO timestamp") @@ -138,10 +146,12 @@ export function registerMessagingCommands(program: Command): void { .option("--verbose", "Show full message bodies") .option("-j, --json", "Output as JSON") .action(async (opts) => { + const senderFilter = resolveSenderFilter(opts); + if (senderFilter.viaFromAlias) noteSenderFilterAlias(senderFilter.sender as string); const window = getCliWindow({ limit: opts.limit, cursor: opts.cursor }); const query = { session_id: opts.session, - from: opts.from, + from: senderFilter.sender, to: opts.to, channel: opts.channel, since: opts.since, @@ -160,6 +170,16 @@ export function registerMessagingCommands(program: Command): void { if (ids.length > 0) await await getStore().markReadByIds(ids, reader); } + if (messages.length === 0) { + discloseEmptyResult({ + channel: opts.channel, + sender: senderFilter.sender, + to: opts.to, + session: opts.session, + since: opts.since, + }, { senderFlag: senderFilter.flag }); + } + if (opts.json) { printJson(messages); warnIfPageFull(messages.length, query.limit); @@ -292,7 +312,8 @@ export function registerMessagingCommands(program: Command): void { .description("Search messages by content") .argument("", "Search query string") .option("--channel ", "Filter by channel") - .option("--from ", "Filter by sender") + .option("--sender ", SENDER_HELP) + .option("--from ", FROM_ALIAS_HELP) .option("--to ", "Filter by recipient") .option("--limit ", "Max results to return (the server caps a single page at 500)", parseInt) .option("--cursor ", "Skip first N results for pagination", parseInt) @@ -325,12 +346,26 @@ channel, which is an ABSENCE claim. together. To enumerate a sender exhaustively, page a listing verb; do not infer a - population from a content search.`) + population from a content search. + + 3. --from IS A SENDER FILTER HERE, NOT YOUR IDENTITY. On nearly every other + subcommand --from names the caller; on search, read and export it appends + "AND from_agent = " to your query. So the liveness probe + + conversations search --channel --from + + is unsatisfiable by construction — a dispatched sub-agent is a DIFFERENT + sender, so the one message you are looking for is the one the filter + removes. It answered "No messages found." at rc=0 with an empty stderr + (todos 807d355d). --from still filters, and now always says so; --sender is + the unambiguous spelling. For identity, set CONVERSATIONS_AGENT_ID.`) .action(async (query, opts) => { const q = typeof query === "string" ? query.trim() : ""; if (!q) { emitCliError("Search query cannot be empty.", opts); } + const senderFilter = resolveSenderFilter(opts); + if (senderFilter.viaFromAlias) noteSenderFilterAlias(senderFilter.sender as string); const window = getCliWindow({ limit: opts.limit, cursor: opts.cursor }); // The store pages this verb now. `--json` used to pass the raw limit and @@ -339,11 +374,19 @@ channel, which is an ABSENCE claim. const result = await getStore().searchMessagesPage({ query: q, channel: opts.channel, - from: opts.from, + from: senderFilter.sender, to: opts.to, limit: opts.json ? opts.limit : window.limit, offset: opts.json ? opts.cursor : window.offset, }); + if (result.items.length === 0) { + discloseEmptyResult({ + query: q, + channel: opts.channel, + sender: senderFilter.sender, + to: opts.to, + }, { senderFlag: senderFilter.flag }); + } const disclosure = { shown: result.items.length, hasMore: result.has_more, @@ -576,20 +619,43 @@ channel, which is an ABSENCE claim. .description("Export messages as JSON or CSV") .option("--channel ", "Filter by channel") .option("--session ", "Filter by session ID") - .option("--from ", "Filter by sender") + .option("--sender ", SENDER_HELP) + .option("--from ", FROM_ALIAS_HELP) .option("--since ", "Messages after this ISO date") .option("--until ", "Messages before this ISO date") .option("--format ", "Output format: json or csv", "json") .action(async (opts) => { + const senderFilter = resolveSenderFilter(opts); + if (senderFilter.viaFromAlias) noteSenderFilterAlias(senderFilter.sender as string); const format = opts.format === "csv" ? "csv" : "json"; const result = await getStore().exportMessages({ channel: opts.channel, session_id: opts.session, - from: opts.from, + from: senderFilter.sender, since: normalizeSince(opts.since), until: opts.until, format, }); + + // An export emptied by the caller's own filter is the same silent false + // absence this change exists to remove, and it reached review as a live + // defect: `export --sender ` printed "[]" with 0 bytes on stderr + // at rc=0, which made --sender on this verb strictly MORE silent than the + // --from it is offered as an improvement on. Emptiness is read off the + // rendered payload rather than re-querying: "[]" for json, headers with no + // data row for csv. + const exportedNothing = format === "csv" + ? !result.includes("\n") + : result.trim() === "[]"; + if (exportedNothing) { + discloseEmptyResult({ + channel: opts.channel, + sender: senderFilter.sender, + session: opts.session, + since: opts.since, + }, { senderFlag: senderFilter.flag }); + } + printLine(result); closeDb(); }); diff --git a/src/cli/sender-filter-disclosure.e2e.test.ts b/src/cli/sender-filter-disclosure.e2e.test.ts new file mode 100644 index 0000000..113a125 --- /dev/null +++ b/src/cli/sender-filter-disclosure.e2e.test.ts @@ -0,0 +1,336 @@ +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import { unlinkSync } from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; + +/** + * End-to-end cover for todos 807d355d (`search`) and e60b8820 (`read`) — the + * same defect on two subcommands. + * + * `--from` means CALLER IDENTITY on ~20 of the CLI's subcommands and SENDER + * FILTER on exactly three (`read`, `search`, `export`). A caller who learned the + * dominant meaning and writes the canonical liveness probe + * + * conversations search --channel --from + * + * gets a SQL predicate `AND m.from_agent = ` bolted onto their query. The + * probe exists to ask "did my sub-agent post?", and a sub-agent is by definition + * a different sender — so the query is structurally unsatisfiable, and it + * answers "No messages found." at rc=0 with an EMPTY stderr. Measured on the + * live store at 0.5.22 before this change: the flag form returned 0 rows while + * the identical query without `--from` returned message #661877. + * + * The fix does NOT flip the filter's meaning. Silently widening a filter into a + * no-op is the same defect pointed the other way — the caller then gets every + * sender's rows at rc=0 and reads it as their own. What changes is that the + * filter is no longer SILENT, and that an unambiguous spelling exists. + * + * Every case below is asserted in both directions. A disclosure that never + * appears and one that always appears are equally worthless, so the negative + * cases — no filter, and a filter that legitimately matches nothing — are as + * load-bearing as the positive ones. + */ + +const TEST_DB = join(tmpdir(), `conversations-sender-filter-${Date.now()}.db`); +const CLI = ["bun", "run", "./src/cli/index.tsx"]; + +/** The measured scenario: a coordinator probing for a sub-agent's post. */ +const CHANNEL = "senderfilter-probe"; +const TOKEN = "subagenttoken807"; +const SUBAGENT = "subagent-807"; +const COORDINATOR = "coordinator-807"; + +function runCli(args: string[], agent: string) { + const result = Bun.spawnSync({ + cmd: [...CLI, ...args], + cwd: process.cwd(), + env: { + ...process.env, + // Precedence rule 1 in src/lib/store/index.ts: an explicit DB path wins + // over an exported cloud mode, so fleet credentials in the ambient + // environment cannot pull this suite onto the production store. Verified + // by measurement, not assumed — see the task record. + CONVERSATIONS_DB_PATH: TEST_DB, + CONVERSATIONS_AGENT_ID: agent, + FORCE_COLOR: "0", + }, + stdout: "pipe", + stderr: "pipe", + }); + return { + exitCode: result.exitCode, + stdout: new TextDecoder().decode(result.stdout), + stderr: new TextDecoder().decode(result.stderr), + /** Callers see one terminal; assert against both streams together. */ + get output() { + return `${new TextDecoder().decode(result.stdout)}${new TextDecoder().decode(result.stderr)}`; + }, + }; +} + +describe("sender filter is never silent", () => { + beforeAll(() => { + // A channel send is refused outright if the channel does not exist. + const created = runCli(["channel", "create", CHANNEL], COORDINATOR); + expect(created.exitCode).toBe(0); + + // The sub-agent posts the token the coordinator is looking for... + const sub = runCli( + ["send", `[${TOKEN}] sub-agent reporting in`, "--channel", CHANNEL], + SUBAGENT, + ); + expect(sub.exitCode).toBe(0); + + // ...and the coordinator has its own unrelated post in the same channel, + // so "0 rows" cannot be explained by an empty channel. + const coord = runCli( + ["send", "dispatch record, no token here", "--channel", CHANNEL], + COORDINATOR, + ); + expect(coord.exitCode).toBe(0); + }, 30_000); + + afterAll(() => { + for (const suffix of ["", "-wal", "-shm"]) { + try { unlinkSync(`${TEST_DB}${suffix}`); } catch {} + } + }); + + // ---- the message is reachable at all (fixture control) ---- + + test("CONTROL: without a sender filter the sub-agent's post is found", () => { + const res = runCli(["search", TOKEN, "--channel", CHANNEL], COORDINATOR); + expect(res.exitCode).toBe(0); + expect(res.stdout).toContain(SUBAGENT); + }); + + // ---- POSITIVE: findable via an unambiguous spelling ---- + + test("--sender selects the sub-agent's post", () => { + const res = runCli( + ["search", TOKEN, "--channel", CHANNEL, "--sender", SUBAGENT], + COORDINATOR, + ); + expect(res.exitCode).toBe(0); + expect(res.stdout).toContain(SUBAGENT); + }); + + test("--sender works on read as well as search", () => { + const res = runCli( + ["read", "--channel", CHANNEL, "--sender", SUBAGENT], + COORDINATOR, + ); + expect(res.exitCode).toBe(0); + expect(res.stdout).toContain(SUBAGENT); + }); + + // ---- POSITIVE: the false absence is disclosed ---- + + test("search: a zero caused by --from names the sender filter", () => { + const res = runCli( + ["search", TOKEN, "--channel", CHANNEL, "--from", COORDINATOR], + COORDINATOR, + ); + // The filter still filters — the row genuinely does not match. + expect(res.exitCode).toBe(0); + expect(res.stdout).not.toContain(SUBAGENT); + // ...but the caller is told WHY, and told that --from is a sender filter + // here rather than their identity. + expect(res.output).toContain("sender"); + expect(res.output).toContain(COORDINATOR); + expect(res.output.toLowerCase()).toContain("--sender"); + }); + + test("read: a zero caused by --from names the sender filter", () => { + const res = runCli( + ["read", "--channel", CHANNEL, "--from", "nobody-sent-anything"], + COORDINATOR, + ); + expect(res.exitCode).toBe(0); + expect(res.output).toContain("sender"); + expect(res.output).toContain("nobody-sent-anything"); + }); + + test("--from is disclosed even when it returns rows, because a filtered non-zero is wrong too", () => { + // The measured case included a NON-empty wrong answer: `--from manius` + // returned manius's own row and hid the sub-agent's. Warning only on zero + // would leave that case silent. + const res = runCli(["search", TOKEN, "--from", SUBAGENT], COORDINATOR); + expect(res.exitCode).toBe(0); + expect(res.stdout).toContain(SUBAGENT); + expect(res.stderr.toLowerCase()).toContain("--sender"); + }); + + // ---- NEGATIVE: the disclosure is not unconditional ---- + + test("an unfiltered zero does NOT claim a sender filter was applied", () => { + const res = runCli( + ["search", "zzqxnotarealtokenhere", "--channel", CHANNEL], + COORDINATOR, + ); + expect(res.exitCode).toBe(0); + expect(res.output).not.toContain("--sender"); + // A second assertion here used to check for the absence of the string + // "filtered by sender", which appears nowhere in src/ except that assertion + // — so it could not fail in any state. Removed rather than reworded: an + // assertion that cannot fail is exactly the defect class this suite is about. + expect(res.output).not.toContain("sender="); + }); + + test("a clean unfiltered search does NOT emit the alias note", () => { + const res = runCli(["search", TOKEN, "--channel", CHANNEL], COORDINATOR); + expect(res.exitCode).toBe(0); + expect(res.stderr).not.toContain("--sender"); + }); + + // ---- NEGATIVE: the filter still filters ---- + + test("--sender is a real filter, not silently widened into a no-op", () => { + const res = runCli( + ["search", TOKEN, "--channel", CHANNEL, "--sender", COORDINATOR], + COORDINATOR, + ); + expect(res.exitCode).toBe(0); + expect(res.stdout).not.toContain(SUBAGENT); + expect(res.stdout).toContain("No messages found."); + }); + + test("--from keeps its existing filter semantics exactly", () => { + // Existing callers must not silently start receiving other senders' rows. + const res = runCli(["search", TOKEN, "--from", COORDINATOR], COORDINATOR); + expect(res.exitCode).toBe(0); + expect(res.stdout).not.toContain(SUBAGENT); + }); + + // ---- conflicting spellings never guess ---- + + test("--from and --sender disagreeing is a hard error, not a silent winner", () => { + const res = runCli( + ["search", TOKEN, "--from", COORDINATOR, "--sender", SUBAGENT], + COORDINATOR, + ); + expect(res.exitCode).toBe(1); + // Naming BOTH values is what distinguishes a real conflict error from + // commander's generic "unknown option '--sender'", which also exits 1 and + // would let this test pass against an unfixed build. + expect(res.output).toContain(COORDINATOR); + expect(res.output).toContain(SUBAGENT); + }); + + test("--from and --sender agreeing is accepted", () => { + const res = runCli( + ["search", TOKEN, "--from", SUBAGENT, "--sender", SUBAGENT], + COORDINATOR, + ); + expect(res.exitCode).toBe(0); + expect(res.stdout).toContain(SUBAGENT); + }); + + // ---- export: the third sender-filtered verb ---- + + test("export: a zero caused by --sender names the sender filter", () => { + // Found in adversarial review: --sender on export was strictly MORE silent + // than the --from it is offered as an improvement on, because it gets no + // alias note by design and had no empty-result disclosure either. It printed + // "[]" with 0 bytes of stderr at rc=0. + const res = runCli( + ["export", "--channel", CHANNEL, "--sender", "ghostsender"], + COORDINATOR, + ); + expect(res.exitCode).toBe(0); + expect(res.stdout.trim()).toBe("[]"); + expect(res.stderr).toContain("ghostsender"); + expect(res.stderr).toContain("--sender"); + }); + + test("export: a NON-empty result stays silent, so the notice means something", () => { + const res = runCli( + ["export", "--channel", CHANNEL, "--sender", SUBAGENT], + COORDINATOR, + ); + expect(res.exitCode).toBe(0); + expect(res.stdout).toContain(SUBAGENT); + expect(res.stderr).toBe(""); + }); + + test("export: csv format discloses an empty export too", () => { + const res = runCli( + ["export", "--channel", CHANNEL, "--sender", "ghostsender", "--format", "csv"], + COORDINATOR, + ); + expect(res.exitCode).toBe(0); + expect(res.stderr).toContain("ghostsender"); + }); + + // ---- a blank sender is refused, never silently widened ---- + + test("--sender '' is a hard error, not an unfiltered read of the whole channel", () => { + // The dangerous direction: `--sender "$WHO"` with WHO unset would otherwise + // return every sender's messages at rc=0 and read as one sender's. + const res = runCli(["read", "--channel", CHANNEL, "--sender", ""], COORDINATOR); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain(SUBAGENT); + expect(res.output).toContain("--sender"); + }); + + test("--from with a whitespace-only value is refused rather than dropped", () => { + const res = runCli(["read", "--channel", CHANNEL, "--from", " "], COORDINATOR); + expect(res.exitCode).toBe(1); + expect(res.stdout).not.toContain(SUBAGENT); + }); + + test("a padded sender value still filters, rather than erroring", () => { + // Trimming a real value is the beneficial half and must survive the guard. + const res = runCli( + ["search", TOKEN, "--channel", CHANNEL, "--sender", ` ${SUBAGENT} `], + COORDINATOR, + ); + expect(res.exitCode).toBe(0); + expect(res.stdout).toContain(SUBAGENT); + }); + + // ---- the empty-result hint echoes the caller's own spelling ---- + + test("the hint names --from when the caller passed --from", () => { + const res = runCli( + ["search", TOKEN, "--channel", CHANNEL, "--from", "ghostsender"], + COORDINATOR, + ); + expect(res.exitCode).toBe(0); + expect(res.stderr).toContain("drop --from to search all senders"); + }); + + test("the hint names --sender when the caller passed --sender", () => { + const res = runCli( + ["search", TOKEN, "--channel", CHANNEL, "--sender", "ghostsender"], + COORDINATOR, + ); + expect(res.exitCode).toBe(0); + expect(res.stderr).toContain("drop --sender to search all senders"); + }); + + // ---- the --json stdout contract is unchanged ---- + + test("--json: stdout stays a bare array and the disclosure goes to stderr", () => { + const res = runCli( + ["search", TOKEN, "--channel", CHANNEL, "--from", COORDINATOR, "--json"], + COORDINATOR, + ); + expect(res.exitCode).toBe(0); + const rows = JSON.parse(res.stdout); + expect(Array.isArray(rows)).toBe(true); + expect(rows).toHaveLength(0); + // Every monitor on this fleet parses stdout as an array, so the disclosure + // must not land there. + expect(res.stderr.toLowerCase()).toContain("--sender"); + }); + + // ---- help says what the flag does ---- + + test("search --help distinguishes the sender filter from caller identity", () => { + const res = runCli(["search", "--help"], COORDINATOR); + expect(res.exitCode).toBe(0); + expect(res.stdout).toContain("--sender"); + expect(res.stdout).toContain("identity"); + }); +}); diff --git a/src/cli/sender-filter.ts b/src/cli/sender-filter.ts new file mode 100644 index 0000000..b84c73b --- /dev/null +++ b/src/cli/sender-filter.ts @@ -0,0 +1,206 @@ +import chalk from "chalk"; +import { printErrorLine } from "../lib/stdout.js"; +import { emitCliError } from "./cli-error.js"; + +/** + * `--from` means two opposite things in this CLI, and the collision is silent. + * + * On roughly twenty subcommands — `send`, `reply`, `edit`, `delete`, `blockers`, + * `notifications`, `watch`, `digest --mark-read`, and every `agents`, + * `analytics`, `locks`, `channel` and `project` verb — `--from` names the CALLER: + * who you are. On exactly three — `read`, `search`, `export` — it is a FILTER on + * `m.from_agent`: who sent the message. Same spelling, opposite meaning, no + * warning either way. + * + * The cost is not hypothetical. The canonical liveness probe a coordinator uses + * to ask "did my dispatched sub-agent post its token?" is + * + * conversations search --channel --from + * + * written that way because `--from` is identity nearly everywhere else. A + * sub-agent is by definition a DIFFERENT sender, so the appended predicate makes + * the query unsatisfiable by construction: it can only ever return the + * coordinator's own dispatch record. Measured against the live store at 0.5.22, + * the flag form printed "No messages found." with an EMPTY stderr at rc=0, while + * the identical query without `--from` returned message #661877 (todos 807d355d; + * the same shape on `read` is e60b8820). + * + * WHAT THIS MODULE DOES NOT DO, deliberately: it does not flip `--from` to mean + * identity on these three verbs. Two reasons, and the second is the load-bearing + * one. + * + * 1. On `search` and `export` no identity is resolved at all, so "identity" + * would make the flag a silent no-op — a flag that accepts a value and + * ignores it is the same defect wearing different clothes. + * 2. It would silently WIDEN every existing caller's result set. A script + * auditing "messages from X" would start receiving every sender's rows at + * rc=0 and read them as X's. This fleet has already measured that direction + * (`todos list --assigned ''` returning the entire store) and it is the more + * dangerous one for automation: a wrong-empty is noticed, a wrong-full is + * acted upon. + * + * So the filter keeps its meaning and stops being silent. `--sender` is the + * unambiguous spelling; `--from` remains a working alias that always announces + * what it did; and a zero produced by any filter says which filters produced it. + */ + +/** Options shape shared by every sender-filtered verb. */ +export interface SenderFilterOptions { + from?: unknown; + sender?: unknown; + json?: boolean; + contract?: boolean; +} + +export interface ResolvedSenderFilter { + /** The sender to filter on, or undefined when no filter was requested. */ + sender: string | undefined; + /** True when the caller spelled it `--from`, which needs the alias note. */ + viaFromAlias: boolean; + /** The spelling the caller actually typed, for messages that echo it back. */ + flag: string; +} + +function trimmed(value: unknown): string | undefined { + if (typeof value !== "string") return undefined; + const text = value.trim(); + return text ? text : undefined; +} + +/** The flag was present on the command line but carries nothing usable. */ +function providedButBlank(value: unknown): boolean { + return typeof value === "string" && value.trim() === ""; +} + +/** + * Resolve `--sender` / `--from` into one sender filter. + * + * Two different values is a hard error rather than a precedence rule. A caller + * who passes both plainly holds two beliefs about what the flags mean, and + * picking a winner silently would resolve that disagreement in favour of + * whichever one the implementer happened to order first — invisibly, in the one + * situation where the caller's confusion is already proven. + */ +export function resolveSenderFilter(opts: SenderFilterOptions): ResolvedSenderFilter { + // A PRESENT but blank value is an error, never "no filter". + // + // `--sender "$WHO"` with WHO unset would otherwise drop the predicate and + // return the ENTIRE channel at rc=0 in silence — the direction this file + // argues is the more dangerous one, arriving through the flag added to fix + // it. It is also a regression against the pre-change behaviour: `--from " "` + // used to filter on the literal whitespace and return nothing, so trimming it + // away silently converts a wrong-empty into a wrong-full. Both spellings are + // rejected rather than guessed. + if (providedButBlank(opts.sender) || providedButBlank(opts.from)) { + const flag = providedButBlank(opts.sender) ? "--sender" : "--from"; + emitCliError( + `${flag} was given an empty value. A blank sender is refused rather than ignored, because dropping ` + + `the filter would return every sender's messages at exit code 0 and read as one sender's. ` + + `Pass a sender name, or omit ${flag} entirely to search all senders.`, + opts, + ); + } + + const fromValue = trimmed(opts.from); + const senderValue = trimmed(opts.sender); + + if (fromValue && senderValue && fromValue !== senderValue) { + emitCliError( + `--from ${fromValue} and --sender ${senderValue} disagree about which sender to filter on. ` + + `On this subcommand --from is an alias for --sender (a filter on who SENT the message), ` + + `not your caller identity. Pass one of them, and set CONVERSATIONS_AGENT_ID for identity.`, + opts, + ); + } + + // `--sender` is the spelling to echo whenever it was given, even alongside an + // agreeing `--from`, because it is the one this CLI wants callers to keep. + return { + sender: senderValue ?? fromValue, + viaFromAlias: Boolean(fromValue), + flag: senderValue ? "--sender" : "--from", + }; +} + +/** + * Announce, on stderr, that `--from` was applied as a SENDER FILTER. + * + * Emitted on EVERY use, not only when the result is empty. The measured defect + * included a NON-empty wrong answer — `--from manius` returned manius's own row + * and hid the sub-agent's — so a note that fired only on zero would have stayed + * silent through exactly the case where the caller reads a plausible result and + * never re-checks it. + * + * stderr rather than stdout so the `--json` array and the text result stay byte + * compatible for existing readers, while anyone at a terminal still sees it. + */ +export function noteSenderFilterAlias(sender: string): void { + printErrorLine( + chalk.yellow( + `Note: --from was applied as a SENDER filter (from_agent=${sender}), not as your caller identity. ` + + `On read/search/export --from selects who SENT a message; on every other subcommand it sets who you are. ` + + `Use --sender ${sender} to say so unambiguously, and CONVERSATIONS_AGENT_ID to set identity.`, + ), + ); +} + +export interface AppliedFilters { + query?: string; + channel?: string; + sender?: string; + to?: string; + session?: string; + since?: string; +} + +/** "query=\"tok\", channel=ops, sender=alice" — only the filters actually set. */ +export function formatAppliedFilters(filters: AppliedFilters): string { + const parts: string[] = []; + if (filters.query) parts.push(`query="${filters.query}"`); + if (filters.channel) parts.push(`channel=${filters.channel}`); + if (filters.sender) parts.push(`sender=${filters.sender}`); + if (filters.to) parts.push(`to=${filters.to}`); + if (filters.session) parts.push(`session=${filters.session}`); + if (filters.since) parts.push(`since=${filters.since}`); + return parts.join(", "); +} + +/** + * Make an empty result legible by naming the filters that produced it. + * + * A bare "No messages found." cannot distinguish "this store holds no such + * message" from "your own filter excluded it", and those two facts lead to + * opposite actions — the first ends a search, the second is a query bug. Nothing + * is emitted when no filter was applied, because a disclosure that always + * appears carries no information and gets tuned out. + * + * Goes to stderr in both output modes: the text surface's stdout stays exactly + * "No messages found." for anything matching on it, and the `--json` surface's + * stdout stays a bare array. + */ +export function discloseEmptyResult( + filters: AppliedFilters, + opts: { senderFlag?: string } = {}, +): void { + const applied = formatAppliedFilters(filters); + if (!applied) return; + + const lines = [`No matches. Filters applied: ${applied}.`]; + if (filters.sender) { + // Name the spelling the caller actually typed. Telling someone who passed + // --from to "drop --sender" reads as advice about a flag they did not use. + const flag = opts.senderFlag ?? "--sender"; + lines.push( + `A sender filter excludes every message sent by anyone else, including a sub-agent you dispatched — ` + + `drop ${flag} to search all senders.`, + ); + } + printErrorLine(chalk.dim(lines.join(" "))); +} + +/** Help text for `--from` wherever it is a sender filter rather than identity. */ +export const FROM_ALIAS_HELP = + "Alias for --sender: filter by who SENT the message, NOT your caller identity (use CONVERSATIONS_AGENT_ID for that)"; + +/** Help text for the unambiguous spelling. */ +export const SENDER_HELP = "Filter by who SENT the message";