From 286123a792e6ce14adfdb51243b043f56e310e36 Mon Sep 17 00:00:00 2001 From: colinds <90475914+colinds@users.noreply.github.com> Date: Tue, 8 Sep 2026 13:24:06 -0700 Subject: [PATCH 1/3] Simplify tools and add meal ratings --- CLAUDE.md | 33 +++- README.md | 26 ++- scripts/smoke.ts | 15 +- skills/forkable/SKILL.md | 32 +++- src/config.ts | 11 -- src/index.ts | 3 +- src/net/client.ts | 49 +---- src/order/format.ts | 14 -- src/order/guards.ts | 35 +--- src/order/selections.ts | 14 +- src/order/status.ts | 66 +++---- src/order/types.ts | 174 ++++------------- src/server.ts | 12 +- src/tools.ts | 384 ++++++++++++++++--------------------- src/write-gate.ts | 16 +- tests/client.test.ts | 13 -- tests/order.test.ts | 296 +++++----------------------- tests/tools-rating.test.ts | 343 +++++++++++++++++++++++++++++++++ tests/tools-read.test.ts | 33 +--- tests/tools-write.test.ts | 46 +---- tests/write-gate.test.ts | 58 +++--- 21 files changed, 790 insertions(+), 883 deletions(-) delete mode 100644 src/config.ts create mode 100644 tests/tools-rating.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 6c80801..0a3a57a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -60,9 +60,9 @@ mutation; the caller receives a fresh preview when confirmation can no longer be `ForkableClient` selects retry behavior from the method being called. Never infer operation type by parsing GraphQL text. -- `gql`, `gqlPublic`, and `query` use the query path. A query may retry once after a transport failure +- `gql` and `query` use the query path. A query may retry once after a transport failure or HTTP 5xx. Callers must use these methods only for reads; retry safety depends on that contract. -- `gqlRaw` and `mutate` use the mutation path. A mutation is never retried after a transport failure, +- `mutate` uses the mutation path. A mutation is never retried after a transport failure, redirect, HTTP 408/5xx, malformed successful response, top-level execution failure with ambiguous data, or missing/malformed mutation payload. - The only mutation replay is one retry after an actual first HTTP `419`, following a fresh CSRF @@ -127,7 +127,6 @@ require positive ownership: `piece.userId` must equal the effective `me.id`. - `mode: "add"` always uses `addPiece` without resolving a source piece. It cannot be combined with `sourcePieceId`, and its input includes `userId` and `replacedPieceId: null` but no `oldPieceId`. - `remove_meal` requires a unique id and positive ownership. -- `skip_delivery` operates only when exactly one owned piece can be resolved. - `set_meal_all` deduplicates delivery ids and refuses a target day with multiple owned pieces; those days must be handled individually. @@ -142,6 +141,26 @@ fields has returned HTTP 503. Keep `confirmDelivery` on its known selection. `replaceAllPieces.newPiece.deliveryId` is the first target delivery id, and its payload selection is `errors`. +## Meal ratings + +`rate_meal` uses the authenticated dashboard's `rateMeal` mutation with selection `errors`. +Resolve `deliveryId` and a unique, positively owned `pieceId`, then send `piece.userRating.id` as +`id` with `channel: "mc"`. A missing rating record or id is unavailable; never invent one. Buffet +ratings use a different flow and are unsupported here. + +Scores are integers from 1–5. Levels 4–5 use the dashboard's compliment codes; 1–3 use its issue +codes. Explicit incompatible reasons are rejected. A category change filters incompatible stored +reasons. Omitted feedback preserves existing values; explicit empty reasons or comments clear them. +Preserve existing attachments, and do not call `updateUser` or invent follow-up consent. + +Read projections expose nullable `rating` objects with level, reasons, comment, guest flag, and +follow-up preference. No record means unavailable; a record without a level means unrated. Mutation +IDs, channel, and attachments stay internal. Ratings and meals always use the same ownership filter. + +Rating previews search from 14 days ago by default and accept `from` for older meals. The stored +plan includes `reconciliationRange`; an uncertain outcome returns it as `reconciliation.arguments` +for `list_deliveries`. Preserve this range through the gate's structured clone and confirmation. + ## `selectionsHash` Customization is keyed by modifier id, with arrays of option ids: @@ -157,8 +176,8 @@ String choices resolve only by a unique trimmed, case-insensitive match: modifie are blocking selection violations. Numeric ids remain the preferred unambiguous input. Explicit emptiness is not absence. An explicitly empty optional single stays `[-1]`; an explicitly -empty required modifier violates the requirement. An absent choice may preserve stored selections or -use the API-ordered first option for a required default. Do not invent diet-aware defaults locally. +empty required modifier violates the requirement. An absent choice uses the API-ordered first option +for a required default. Do not invent diet-aware defaults locally. ## Thin-client validation @@ -270,5 +289,5 @@ bun run smoke binary without credentials. Keep `scripts/` in the TypeScript project. Use TypeScript strict mode, two-space indentation, kebab-case files, and snake_case tool names. Reads -use `get_`, `list_`, `search_`, `recommend_`, or `explain_`; writes use `set_`, `remove_`, `skip_`, or -`confirm_` and accept an optional `confirmToken`. +use `get_`, `list_`, `search_`, or `recommend_`; writes use `set_`, `remove_`, `confirm_`, or `rate_` +and accept an optional `confirmToken`. diff --git a/README.md b/README.md index 2b1b484..289bd44 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,8 @@ lunch is without opening the Forkable app. - Track courier ETAs, arrival times, and office access notes - Browse and search menus - Get Forkable's meal recommendations -- Add, replace, remove, skip, and confirm meals +- Add, replace, remove, and confirm meals +- Rate meals from 1–5 and leave written feedback - Use it from any MCP client that can launch a stdio server ## Quick start @@ -83,17 +84,36 @@ The source files are in [`skills/`](./skills). | `get_menus` | Lists menus and item options for a delivery | | `search_items` | Searches a delivery's menus | | `recommend_meals` | Returns Forkable's meal recommendations | -| `explain_pick` | Shows where the current meal appears in Forkable's recommendations | | `get_profile` | Shows the signed-in Forkable user | | `set_meal` | Adds or replaces a meal | | `set_meal_all` | Sets the same meal on several deliveries | | `remove_meal` | Removes a meal | -| `skip_delivery` | Skips a delivery | +| `rate_meal` | Rates a meal or edits its score and text feedback | | `confirm_delivery` | Confirms or unconfirms a delivery | Forkable still decides whether a change is allowed, including deadlines, restaurant capacity, and billing rules. +To skip a delivery, use `list_deliveries` and remove each selected owned meal with `remove_meal`. +This removes those meals; it does not change your auto-order settings. Compare selected meals with +`recommend_meals` when choosing alternatives. + +### Meal ratings + +List the delivery first, supplying `from` and `to` for past meals. Each owned meal includes `rating`: +`null` means Forkable has not made a rating available, while a rating with `level: null` is unrated. + +Use `rate_meal` with the delivery ID, the meal's `pieceId`, and a `level` from 1–5. It previews first; +call again with the same arguments and its `confirmToken` to submit. The search starts 14 days ago +by default; pass `from` to rate an older meal. + +Optional `reasons`, `comment`, `forGuest`, and `allowRatingFollowUps` edit the feedback. Omitted fields +keep their current values; an empty comment or reason array clears it. Levels 4–5 accept compliment +codes, while 1–3 accept issue codes listed in the tool schema. Changing score categories removes +incompatible stored reasons. Marking a meal as a guest meal excludes its rating from your future +suggestions. Follow-up preferences apply to this rating without changing your account settings. +Existing attachments are kept; photo editing and buffet ratings are not supported. + ## Authentication There is no API key. The server reuses a Forkable web session and stores it in diff --git a/scripts/smoke.ts b/scripts/smoke.ts index a742b1c..2c8eb25 100644 --- a/scripts/smoke.ts +++ b/scripts/smoke.ts @@ -22,17 +22,16 @@ const exec = promisify(execFile); const EXPECTED_TOOLS = [ "confirm_delivery", - "explain_pick", "get_delivery_status", "get_menus", "get_profile", "list_deliveries", + "rate_meal", "recommend_meals", "remove_meal", "search_items", "set_meal", "set_meal_all", - "skip_delivery", ]; const log = (msg: string) => console.log(` ${msg}`); @@ -120,6 +119,18 @@ async function checkInstalled(runner: Runner, cwd: string, home: string): Promis } log(`${runner} write schemas require exact menu identity and expose additional meals`); + const rating = listedTools.find((tool) => tool.name === "rate_meal"); + const ratingSchema = rating?.inputSchema as + | { required?: string[]; properties?: Record } + | undefined; + for (const name of ["deliveryId", "pieceId", "level"]) { + if (!ratingSchema?.required?.includes(name)) + fail(`${runner}: rate_meal does not require ${name}`); + } + if (!ratingSchema?.properties?.confirmToken) + fail(`${runner}: rate_meal does not expose confirmToken`); + log(`${runner} rating schema requires an exact meal and score`); + const res: any = await client.callTool({ name: "get_profile", arguments: {} }); const text = (res.content ?? []).map((content: any) => content.text ?? "").join(""); if (!text.trim()) fail(`${runner}: get_profile returned no content`); diff --git a/skills/forkable/SKILL.md b/skills/forkable/SKILL.md index af4bf9a..add6420 100644 --- a/skills/forkable/SKILL.md +++ b/skills/forkable/SKILL.md @@ -1,7 +1,7 @@ --- name: forkable description: >- - Use the forkable MCP server to read, choose, change, skip, confirm, and track meals. Use for + Use the forkable MCP server to read, choose, change, rate, confirm, and track meals. Use for Forkable delivery, menu, recommendation, meal, and courier-status requests, including workflows that also use a more focused Forkable skill. --- @@ -78,13 +78,31 @@ extra meal is covered or will be charged; Forkable decides that. duplicate delivery IDs. A delivery with several owned meals must be handled individually with `set_meal` and `sourcePieceId`. -`remove_meal` requires an owned `pieceId`. `skip_delivery` removes the only positively owned -meal on a delivery; use `remove_meal` separately when more than one is owned. +`remove_meal` requires an owned `pieceId`. To skip a delivery, list it and remove each meal the user +wants removed, using its exact owned `pieceId`. This does not disable auto-ordering for future days. `confirm_delivery` confirms by default. Pass `confirm: false` to unconfirm without removing the meal. `set_meal` can use `autoConfirm` when the user wants the replacement and confirmation in one mutation. +## Rate a meal + +Use `rate_meal` for a 1–5 score or feedback edits on one owned meal. List past deliveries with explicit +`from` and `to`, then use the returned `deliveryId` and `pieceId`. The rating tool searches from 14 +days ago by default; pass `from` for older meals. A meal's `rating: null` means unavailable, while a +rating object with `level: null` means unrated. Do not infer rating availability from delivery status. + +Ask for the user's actual score and feedback; do not manufacture a rating from their food preferences. +Optional reasons use the tool schema's compliment codes for 4–5 and issue codes for 1–3. Omitted +feedback stays unchanged; explicit empty comments or reason arrays clear it. When switching score +categories, incompatible stored reasons are removed. Existing attachments are kept. + +`forGuest: true` excludes this rating from the user's future meal suggestions. Set +`allowRatingFollowUps` only when the user states a preference; it applies to this rating and does not +change account settings. Show the exact score, feedback, and preferences in the preview before +confirming. Use delivery lists and recommendations to compare meals; no tool explains the model's +reasoning or reports ranks beyond the returned recommendations. + ## Dietary advisory While creating a `set_meal` or `set_meal_all` preview, the server calls Forkable's @@ -123,7 +141,8 @@ Review the replacement preview before using its new token. - `rejected`: Forkable definitively refused the write. Stop and report the reasons; do not reuse the consumed token. - `outcome_unknown`: Forkable may have applied the write. Do not retry. Refresh the delivery IDs - named in `reconciliation` with `list_deliveries`, then compare the current state. + named in `reconciliation` with `list_deliveries`, passing `reconciliation.arguments` when present + so historical ratings are included, then compare the current state. Mutations are not retried after an ambiguous transport or server failure. @@ -138,5 +157,6 @@ Quote them as reported. Do not calculate company coverage or an authoritative ou ## Unsupported account actions -The tools do not rate meals, report missing or incorrect items, change vacation settings, edit -Forkable dietary settings, or switch offices. Direct the user to Forkable for those actions. +The tools do not submit buffet ratings, edit rating photos, report missing or incorrect items, +change vacation settings, edit Forkable dietary settings, or switch offices. Direct the user to Forkable +for those actions. diff --git a/src/config.ts b/src/config.ts deleted file mode 100644 index 5ce2d78..0000000 --- a/src/config.ts +++ /dev/null @@ -1,11 +0,0 @@ -import pkg from "../package.json" with { type: "json" }; - -export const VERSION: string = pkg.version; - -export interface Config { - version: string; -} - -export function loadConfig(): Config { - return { version: VERSION }; -} diff --git a/src/index.ts b/src/index.ts index 37c8b9e..6d11a56 100644 --- a/src/index.ts +++ b/src/index.ts @@ -4,12 +4,11 @@ // `bun run src/index.ts` → serve MCP over stdio (client-spawned). import { runStdio } from "./server.ts"; -import { loadConfig } from "./config.ts"; import { runAuthCli } from "@/auth/cli.ts"; const argv = process.argv.slice(2); if (argv.includes("--auth")) { await runAuthCli(argv); } else { - await runStdio(loadConfig()); + await runStdio(); } diff --git a/src/net/client.ts b/src/net/client.ts index 441ea16..5c15673 100644 --- a/src/net/client.ts +++ b/src/net/client.ts @@ -1,12 +1,6 @@ // Authenticated GraphQL client with operation-aware retry and cookie rotation. -import { - ENDPOINT, - PUBLIC_ENDPOINT, - CSRF_URL, - forkableHeaders, - type FetchImpl, -} from "./endpoints.ts"; +import { ENDPOINT, CSRF_URL, forkableHeaders, type FetchImpl } from "./endpoints.ts"; import { ReauthRequiredError, MutationError, @@ -84,17 +78,12 @@ type Operation = "query" | "mutation"; interface RequestOptions { operation: Operation; operationName: string; - public: boolean; queryRetries: number; csrfRetries: number; } -function requestOptions( - operation: Operation, - operationName: string = operation, - isPublic = false, -): RequestOptions { - return { operation, operationName, public: isPublic, queryRetries: 0, csrfRetries: 0 }; +function requestOptions(operation: Operation, operationName: string = operation): RequestOptions { + return { operation, operationName, queryRetries: 0, csrfRetries: 0 }; } function isRecord(value: unknown): value is Record { @@ -206,18 +195,13 @@ export class ForkableClient { variables: Record | undefined, options: RequestOptions, ): Promise> { - if (!options.public && !this.csrf) await this.mintCsrf(); + if (!this.csrf) await this.mintCsrf(); - const endpoint = options.public ? PUBLIC_ENDPOINT : ENDPOINT; - const headers = forkableHeaders( - this.cookie, - options.public ? undefined : this.csrf, - this.delegation, - ); + const headers = forkableHeaders(this.cookie, this.csrf, this.delegation); let res: Response; try { - res = await this.fetchImpl(endpoint, { + res = await this.fetchImpl(ENDPOINT, { method: "POST", redirect: options.operation === "mutation" ? "manual" : "follow", headers, @@ -247,7 +231,7 @@ export class ForkableClient { const setCookies = res.headers.getSetCookie?.() ?? []; if (setCookies.length) await this.persist({ setCookies }); - if (!options.public && res.status === 419) { + if (res.status === 419) { if (options.csrfRetries < 1) { try { await this.mintCsrf(); @@ -380,18 +364,6 @@ export class ForkableClient { return body; } - /** Low-level POST using mutation-safe retry behavior. */ - async gqlRaw( - query: string, - variables?: Record, - o: { public?: boolean; retried?: number } = {}, - ): Promise> { - return this.sendGraphql(query, variables, { - ...requestOptions("mutation", "mutation", o.public ?? false), - csrfRetries: o.retried ?? 0, - }); - } - /** Run a query document, throwing on GraphQL errors; returns `data`. */ async gql(query: string, variables?: Record): Promise { const r = await this.sendGraphql(query, variables, requestOptions("query")); @@ -399,13 +371,6 @@ export class ForkableClient { return (r.data ?? null) as T; } - /** Public (unauthenticated) endpoint — e.g. `identities`, `diets`. */ - async gqlPublic(query: string, variables?: Record): Promise { - const r = await this.sendGraphql(query, variables, requestOptions("query", "query", true)); - if (r.errors?.length) throw new QueryError(r.errors); - return (r.data ?? null) as T; - } - /** Sugar: `query("menus", {ids,clubId}, "id name")` → returns `data.menus`. */ async query( root: string, diff --git a/src/order/format.ts b/src/order/format.ts index dcf246d..18b5fed 100644 --- a/src/order/format.ts +++ b/src/order/format.ts @@ -30,14 +30,6 @@ export function formatDay(iso?: string): string { const valid = (d: Date): Date | undefined => (Number.isNaN(d.getTime()) ? undefined : d); const pad2 = (n: number): string => String(n).padStart(2, "0"); -/** Parse Forkable floating-local timestamps; true offsets remain real instants. */ -export function parseFloating(iso?: string): Date | undefined { - if (!iso) return undefined; - if (/[+-]\d{2}:?\d{2}$/.test(iso)) return valid(new Date(iso)); // genuine offset: a real instant - const local = /\d{2}:\d{2}/.test(iso) ? iso.replace(/Z$/i, "") : `${formatDate(iso)}T00:00:00`; - return valid(new Date(local)); -} - /** Format the wall clock exactly as named by the timestamp, without host-zone conversion. */ export function formatDateTime(iso?: string): string { const m = /^(\d{4}-\d{2}-\d{2})T(\d{2}):(\d{2})/.exec(iso ?? ""); @@ -105,12 +97,6 @@ export function formatCountdown(iso?: string, now: Date = new Date()): string { return h > 0 ? `${h}h ${minutes % 60}m` : `${minutes}m`; } -/** Has this timestamp already passed? `undefined` when there's nothing to compare. */ -export function isPast(iso?: string, now: Date = new Date()): boolean | undefined { - const at = parseFloating(iso); - return at ? at.getTime() < now.getTime() : undefined; -} - /** Format a per-piece dropoff group suffix. */ export function groupSuffix(group?: string | null): string { return group ? ` — group ${group}` : ""; diff --git a/src/order/guards.ts b/src/order/guards.ts index edd4d24..738b6e8 100644 --- a/src/order/guards.ts +++ b/src/order/guards.ts @@ -10,39 +10,18 @@ export interface OwnOrder { pieces: Piece[]; } -export interface OwnMeal { - /** First order carrying matching pieces. */ - order: Order; - pieces: Piece[]; - /** All orders carrying matching pieces. */ - orders: OwnOrder[]; - /** Whether matching pieces span several orders. */ - ambiguous: boolean; - /** Whether matching used a user id. */ - byIdentity: boolean; -} - -/** Match pieces across venue orders. Writes must supply `userId` and reject ambiguous matches. */ -export function findOwnMeal(d: Delivery, userId?: number): OwnMeal | undefined { - const mine: OwnOrder[] = (d.orders ?? []).flatMap((o) => { - const all = o.pieces ?? []; - const ps = userId == null ? all : all.filter((p) => p.userId === userId); - return ps.length ? [{ order: o, pieces: ps }] : []; +/** Match only positively owned pieces, preserving their venue orders. */ +export function ownedOrders(d: Delivery, userId?: number): OwnOrder[] { + if (userId == null) return []; + return (d.orders ?? []).flatMap((order) => { + const pieces = (order.pieces ?? []).filter((piece) => piece.userId === userId); + return pieces.length ? [{ order, pieces }] : []; }); - const first = mine[0]; - if (!first) return undefined; - return { - order: first.order, - pieces: first.pieces, - orders: mine, - ambiguous: mine.length > 1, - byIdentity: userId != null, - }; } /** Every piece the member owns across all venues today. */ export function ownPieces(d: Delivery, userId?: number): Piece[] { - return findOwnMeal(d, userId)?.orders.flatMap((o) => o.pieces) ?? []; + return ownedOrders(d, userId).flatMap((o) => o.pieces); } /** Every piece across all venue orders, including guest picks. */ diff --git a/src/order/selections.ts b/src/order/selections.ts index 3b7a31d..d815086 100644 --- a/src/order/selections.ts +++ b/src/order/selections.ts @@ -76,11 +76,6 @@ export function resolveItemModifiers( return ordered; } -/** The API-ordered default for a required modifier. */ -export function defaultOption(mod: MenuModifier): MenuOption | undefined { - return mod.options[0]; -} - /** Added price (dollars) of an option: its own price, else the modifier's option-set price, else 0. */ function optionPrice(opt: MenuOption, mod: MenuModifier, menu?: Menu): number { if (typeof opt.price === "number") return opt.price; @@ -96,7 +91,6 @@ export interface BuildSelectionsInput { item: MenuItem; modifiers?: MenuModifier[]; // defaults to resolveItemModifiers(item) choices?: ModifierChoice[]; // explicit user choices - previous?: SelectionsHash | null; // an existing piece's stored selections (for round-trip / defaults) } const normalizeName = (value: string): string => value.trim().toLowerCase(); @@ -107,10 +101,9 @@ function resolveUnique(values: T[], name: string, label: (value: T) => string return values.filter((value) => normalizeName(label(value)) === normalized); } -/** Build selections from explicit choices, prior values, or API-ordered defaults. */ +/** Build selections from explicit choices or API-ordered defaults. */ export function buildSelectionsHash(input: BuildSelectionsInput): BuildSelectionsResult { const mods = input.modifiers ?? resolveItemModifiers(input.item); - const prev = input.previous ?? null; const violations: SelectionViolation[] = []; const summary: { modifier: string; options: string[]; extra: number }[] = []; const selectionsHash: SelectionsHash = {}; @@ -190,13 +183,11 @@ export function buildSelectionsHash(input: BuildSelectionsInput): BuildSelection const label = modLabel(mod); const hasUserChoice = chosenByMod.has(mod.id); const user = chosenByMod.get(mod.id); - const previous = prev?.[String(mod.id)]; let selected: number[]; if (isSingleSelect(mod)) { if (hasUserChoice) selected = user?.length ? [user[0]!] : [-1]; - else if (previous?.length) selected = [previous[0]!]; - else selected = [mod.required ? (defaultOption(mod)?.id ?? -1) : -1]; + else selected = [mod.required ? (mod.options[0]?.id ?? -1) : -1]; if (mod.required && selected[0] === -1) { violations.push({ modifierId: mod.id, label, code: "required", selected: 0 }); @@ -212,7 +203,6 @@ export function buildSelectionsHash(input: BuildSelectionsInput): BuildSelection } } else { if (user) selected = user; - else if (previous?.length) selected = previous; else selected = mod.required && mod.options[0] ? [mod.options[0].id] : []; const min = mod.min ?? 0; diff --git a/src/order/status.ts b/src/order/status.ts index 041202d..fe1aa26 100644 --- a/src/order/status.ts +++ b/src/order/status.ts @@ -1,6 +1,7 @@ // Delivery fulfillment projection and rendering. -import { type Delivery, type Order, type Piece } from "./types.ts"; +import { type Delivery, type Order, type UserRating } from "./types.ts"; +import { ownedOrders, type OwnOrder } from "./guards.ts"; import { cancellationPending, formatDate, @@ -16,9 +17,6 @@ import { export interface OwnedOrderStatus { orderId: string | number; venue: string | null; - pieceIds: (string | number)[]; - state: string | null; - etaStatus: string | null; fulfillment: string | null; dropoffCompletedAt: string | null; etaStart: string | null; @@ -50,13 +48,13 @@ export interface DeliveryStatus { name: string; price: number | null; venue: string | null; - autoOrder: boolean | null; options: string[]; group: string | null; isConfirmed: boolean | null; isLateSwappable: boolean | null; cancellationPending: boolean; isLateOrder: boolean | null; + rating: ReturnType; }[]; /** Scheduled service window as Forkable reported it, not an editing window. */ deliveryWindow: string[] | null; @@ -64,17 +62,10 @@ export interface DeliveryStatus { timezone: string | null; address: { formatted: string | null; notes: string | null }; reportMissingItemCutoff: string | null; - reportMissingItemCutoffRaw: string | null; replacementCountdown: string | null; - replacementCutoffRaw: string | null; billing: DeliveryBilling; } -interface OwnedOrder { - order: Order; - pieces: Piece[]; -} - const serviceName = (name: string): string => (name === "afternoon" ? "dinner" : name); function clockOf(iso?: string | null): [string, string] | null { @@ -96,16 +87,6 @@ function dollarsToCents(value: number | null | undefined): number | null { return typeof value === "number" && Number.isFinite(value) ? Math.round(value * 100) : null; } -function positivelyOwnedOrders(d: Delivery, userId?: number): OwnedOrder[] { - if (userId == null) return []; - return (d.orders ?? []).flatMap((order) => { - const pieces = (order.pieces ?? []).filter( - (piece) => piece.userId != null && piece.userId === userId, - ); - return pieces.length ? [{ order, pieces }] : []; - }); -} - function fulfillmentFor(order: Order): string | null { if (order.dropoffCompletedAt) return "delivered"; return order.etaStatus?.status ?? order.state ?? null; @@ -126,28 +107,32 @@ function aggregateFulfillment(orders: OwnedOrderStatus[], d: Delivery): string | return d.simpleState ?? d.state ?? null; } -function soonestReplacement( - orders: OwnedOrder[], - now: Date, -): Pick { - const all = orders +function soonestReplacement(orders: OwnOrder[], now: Date): string | null { + const live = orders .map(({ order }) => order.replacementCutoffTs) - .filter((timestamp): timestamp is string => !!timestamp) - .toSorted((a, b) => Date.parse(a) - Date.parse(b)); - const live = all.find((timestamp) => formatCountdown(timestamp, now) !== ""); + .filter( + (timestamp): timestamp is string => !!timestamp && formatCountdown(timestamp, now) !== "", + ) + .toSorted((a, b) => Date.parse(a) - Date.parse(b))[0]; + return live ? formatCountdown(live, now) : null; +} + +/** Expose feedback without the internal mutation ID or attachment. */ +export function ratingDetails(rating?: UserRating | null) { + if (rating?.id == null || rating.id === "") return null; return { - replacementCountdown: live ? formatCountdown(live, now) : null, - replacementCutoffRaw: live ?? all.at(-1) ?? null, + level: rating.level ?? null, + reasons: rating.reasons ?? [], + comment: rating.comment ?? null, + forGuest: rating.forGuest ?? null, + allowRatingFollowUps: rating.allowRatingFollowUps ?? null, }; } -function orderStatus({ order, pieces }: OwnedOrder): OwnedOrderStatus { +function orderStatus({ order }: OwnOrder): OwnedOrderStatus { return { orderId: order.id, venue: order.venue?.displayName ?? order.venue?.name ?? order.menu?.name ?? null, - pieceIds: pieces.map((piece) => piece.id), - state: order.state ?? null, - etaStatus: order.etaStatus?.status ?? null, fulfillment: fulfillmentFor(order), dropoffCompletedAt: order.dropoffCompletedAt ?? null, etaStart: order.etaStatus?.start ?? null, @@ -162,7 +147,7 @@ export function deliveryStatus( userId?: number, now: Date = new Date(), ): DeliveryStatus { - const owned = positivelyOwnedOrders(d, userId); + const owned = ownedOrders(d, userId); const orders = owned.map(orderStatus); const timezone = d.club?.market?.timezone ?? null; const zoneSource = orders.find((order) => order.etaStart)?.etaStart; @@ -188,7 +173,6 @@ export function deliveryStatus( name: piece.name ?? `item ${piece.itemId}`, price: piece.price ?? null, venue: order.venue?.displayName ?? order.venue?.name ?? order.menu?.name ?? null, - autoOrder: piece.autoOrder ?? null, options: (piece.nonHiddenAttributes ?? []) .map((attribute) => [attribute.label, attribute.value].filter(Boolean).join(": ")) .filter(Boolean), @@ -197,6 +181,7 @@ export function deliveryStatus( isLateSwappable: piece.isLateSwappable ?? null, cancellationPending: cancellationPending(piece), isLateOrder: piece.isLateOrder ?? null, + rating: ratingDetails(piece.userRating), })), ), deliveryWindow: d.deliveryWindow ? [...d.deliveryWindow] : null, @@ -206,8 +191,7 @@ export function deliveryStatus( timezone, address: { formatted: d.address?.formatted ?? null, notes: d.address?.notes ?? null }, reportMissingItemCutoff, - reportMissingItemCutoffRaw: d.reportMissingItemCutoff ?? null, - ...soonestReplacement(owned, now), + replacementCountdown: soonestReplacement(owned, now), billing: { reportedDueCents: dollarsToCents(d.userReceipt?.due), allowanceType: d.allowanceType ?? null, @@ -254,6 +238,8 @@ export function formatDeliveryStatus(s: DeliveryStatus): string { const options = meal.options.length ? ` (${meal.options.join(", ")})` : ""; const segments = [dish + options, meal.venue].filter(Boolean).join(" — "); add("Your meal", segments + groupSuffix(meal.group) + pieceBadges(meal)); + if (meal.rating) + add("Rating", meal.rating.level == null ? "not rated" : `${meal.rating.level}/5`); } if (!s.meal.length) add("Your meal", "— nothing selected"); diff --git a/src/order/types.ts b/src/order/types.ts index e9aeef0..47e7a5d 100644 --- a/src/order/types.ts +++ b/src/order/types.ts @@ -1,10 +1,9 @@ -// Domain fields selected by the tools. +// Domain fields selected by the tools. Monetary values here are dollars. export interface MenuOption { id: number; name: string; - price?: number | null; // dollars (verified: add-ons come back 2.5 / 3.99 / 7.95) - ingredientTags?: string[]; + price?: number | null; } export interface MenuModifier { @@ -24,187 +23,90 @@ export interface MenuItem { menuId: number; name: string; description?: string; - price?: number; // dollars + price?: number; imageUrl?: string; - ingredientTags?: string[]; dietLevel?: number; modifierIds?: number[]; modifiers?: MenuModifier[]; } -export interface MenuSection { - id: number; - name?: string; - description?: string; - items: MenuItem[]; -} - export interface Menu { id: number; name?: string; displayName?: string; - sections: MenuSection[]; - /** Option price fallback; treated as dollars. */ + sections: { items: MenuItem[] }[]; + /** Option price fallback, in dollars. */ optionSets?: { id: number; price?: number | null }[]; - /** This venue takes no custom notes; `instructions` sent anyway are dropped. */ disableSpecialInstructions?: boolean; - venue?: { id: number; name?: string; capacity?: number; familyHub?: boolean }; +} + +/** The dashboard supplies a record even before the first score is submitted. */ +export interface UserRating { + id: string | number; + level?: number | null; + reasons?: string[] | null; + comment?: string | null; + forGuest?: boolean | null; + allowRatingFollowUps?: boolean | null; + attachment?: string | null; } export interface Piece { - id: string | number; // piece ids are UUID strings in practice + id: string | number; itemId: number; menuId: number; - userId?: number; // whose meal — required to tell your piece from a guest's + userId?: number; name?: string; - state?: string; - autoOrder?: boolean; // account auto-order state - /** Forkable flow classification. */ - flowType?: string; - /** Pre-rendered customization labels — cheaper than decoding `selections`. */ - nonHiddenAttributes?: PieceAttribute[]; - instructions?: string; - /** Per-piece dropoff group; null before grouping. */ + nonHiddenAttributes?: { label?: string; value?: string }[]; group?: string | null; - /** Nullable per-piece confirmation, swap, removal, and late-order state. */ isConfirmed?: boolean | null; isLateSwappable?: boolean | null; isRemoval?: boolean | null; requestStatus?: string | null; isLateOrder?: boolean | null; - price?: number; // dollars - selections?: SelectionsHash | null; // stored hash on an existing piece + price?: number; + userRating?: UserRating | null; } -/** Live courier state. Null until dispatched. */ export interface EtaStatus { - start?: string; // true offset — the delivery's zone source + start?: string; // Real offset, also used as a timezone fallback. end?: string; - shortTz?: string; // display label, e.g. "PT" - status?: string; // e.g. "delivered" + shortTz?: string; + status?: string; trackingUrl?: string; } -export interface Dropoff { - id: string | number; - route?: { courierId?: string | number | null; date?: string }; - pickupWindowInfo?: { windowStart?: string; windowEnd?: string }; // explicit offsets -} - -export interface OrderVenue { - id: number; - name?: string; - displayName?: string; - capacity?: number; - /** Family-style venue: meals are shared, so a per-member change request never applies. */ - familyHub?: boolean; -} - -export interface PieceAttribute { - label?: string; - value?: string; -} - -export interface ReportedIssue { - id: string | number; - type?: string; - resolution?: string; - requestReOrder?: boolean; - requestRefund?: boolean; - requestGiftCard?: boolean; - orders?: { id: string | number }[]; - pieces?: { id: string | number }[]; -} - /** One order per venue. Resolve pieces by owner rather than order position. */ export interface Order { id: string | number; state?: string; - isOverVenueCapacity?: boolean; - lateOrdersRemaining?: number; - lateGuestOrdersRemaining?: number; - lateRemovalsRemaining?: number; - changeRequestAllowed?: boolean; - pastLateOrderDeadline?: boolean; - hasVenueLateOrdersRemaining?: boolean; - hasChangeRequest?: boolean; - /** The order this one replaces. */ - replaces?: { id: string | number; menu?: { id: number } }; replacementCutoffTs?: string; - isNextStepsAble?: boolean; - isReorderable?: boolean; - menu?: { id: number; name?: string }; + menu?: { name?: string }; pieces?: Piece[]; - venue?: OrderVenue; + venue?: { name?: string; displayName?: string }; etaStatus?: EtaStatus; - dropoffCompletedAt?: string; // UTC instant - dropoff?: Dropoff; -} - -/** Scheduled service slot, e.g. `{baseTime: "12:00:00", name: "lunch"}`. */ -export interface ServiceWindow { - baseTime?: string; - name?: string; -} - -export interface DeliveryAddress { - street?: string; - city?: string; - postalCode?: string; - formatted?: string; - notes?: string; // building-access instructions + dropoffCompletedAt?: string; // UTC instant. } export interface Delivery { id: number; - state?: string; // ordering lifecycle: "initial" | "grace_period" | "receipt_sent" | … - /** Fulfillment track, orthogonal to `state`. Null until delivered, so use as a fallback. */ + state?: string; // Ordering lifecycle, distinct from fulfillment. simpleState?: string; - forDeliveryAt?: string; // floating local mislabelled UTC — see parseFloating - isReadOnly?: boolean; + forDeliveryAt?: string; // Floating local wall clock despite its Z suffix. userConfirmed?: boolean; - /** Forkable-reported delivery copay, in dollars. */ - copayAmount?: number; // dollars + copayAmount?: number; availableMenuIds?: number[]; - pastLateOrderDeadline?: boolean; - canRequestChanges?: boolean; - /** "daily" | "weekly" | "weekly_by_day" — which of the allowance fields actually applies. */ allowanceType?: string; - weeklyAllowance?: number; // dollars; the weekly cap - weeklyAllowanceAvailable?: number; // dollars; what's left of it. Reads 0 on a daily club. - /** Family-style service flags. */ - forFamily?: boolean | null; + weeklyAllowance?: number; + weeklyAllowanceAvailable?: number; forBuffet?: boolean | null; - deliveryWindow?: string[]; // ["11:45","12:15"] — wall clock, no date, no zone - serviceWindow?: ServiceWindow; - /** Missing-item deadline, rendered only when it is an explicit instant. */ - reportMissingItemCutoff?: string; - address?: DeliveryAddress; - notes?: string; // duplicate of address.notes - club?: { - id: number; - name?: string; - /** Boolean coverage rule, not a count. */ - allowanceMealLimit?: boolean; - allowanceType?: string; - familyHub?: boolean; - isLateRemovalEnabled?: boolean; - market?: { timezone?: string; currencySettings?: { currency?: string } }; - }; + deliveryWindow?: string[]; + serviceWindow?: { baseTime?: string; name?: string }; + reportMissingItemCutoff?: string; // UTC instant. + address?: { formatted?: string; notes?: string }; + club?: { id: number; name?: string; market?: { timezone?: string } }; orders?: Order[]; - myReportedIssues?: ReportedIssue[]; - userReceipt?: { - /** Null until a receipt exists — a future delivery still reports the figures below. */ - id: number | null; - due?: number; - /** Copay applied to this receipt. */ - copayAmount?: number; - /** Forkable-reported member copay context. */ - clubCopay?: number; - subtotal?: number; - feesTotal?: number; - fees?: { type?: string; fee?: number }[]; - }; // all dollars + userReceipt?: { due?: number; clubCopay?: number }; } export type SelectionsHash = Record; diff --git a/src/server.ts b/src/server.ts index db8d95b..cf67932 100644 --- a/src/server.ts +++ b/src/server.ts @@ -3,14 +3,14 @@ import { McpServer } from "@modelcontextprotocol/server"; import { serveStdio } from "@modelcontextprotocol/server/stdio"; import { registerAllTools } from "./tools.ts"; -import { type Config } from "./config.ts"; +import pkg from "../package.json" with { type: "json" }; import { ForkableClient } from "@/net/client.ts"; import { provisionFromEnvIfNeeded } from "@/auth/ingest.ts"; import { createWriteGate } from "@/write-gate.ts"; /** Build a fresh MCP server with all tools registered (one per stdio connection). */ -function makeServer(cfg: Config): McpServer { - const server = new McpServer({ name: "forkable", version: cfg.version }); +function makeServer(): McpServer { + const server = new McpServer({ name: "forkable", version: pkg.version }); registerAllTools(server, createWriteGate()); return server; } @@ -27,15 +27,15 @@ function startKeepalive(): void { ).unref?.(); } -export async function runStdio(cfg: Config): Promise { +export async function runStdio(): Promise { // Best-effort session provisioning from environment credentials. const me = await provisionFromEnvIfNeeded().catch((e) => { console.error(`env auth failed: ${(e as Error).message}`); return null; }); - serveStdio(() => makeServer(cfg)); + serveStdio(makeServer); console.error( - `forkable-mcp v${cfg.version} on stdio${me ? ` (provisioned ${me.email ?? `user ${me.id}`} from env)` : ""}`, + `forkable-mcp v${pkg.version} on stdio${me ? ` (provisioned ${me.email ?? `user ${me.id}`} from env)` : ""}`, ); startKeepalive(); } diff --git a/src/tools.ts b/src/tools.ts index e816c8e..fbc3e9e 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -7,16 +7,10 @@ import { ReauthRequiredError } from "@/net/errors.ts"; import { requireSession, type SessionRecord } from "@/auth/session.ts"; import { loginWithPassword, envLoginInput } from "@/auth/login.ts"; import { buildQuery } from "@/net/gql.ts"; -import { - hashWriteArgs, - type GateCtx, - type WriteGate, - type WritePlan, - type ToolResultLike, -} from "./write-gate.ts"; +import { hashWriteArgs, type GateCtx, type WriteGate, type WritePlan } from "./write-gate.ts"; import { buildSelectionsHash, resolveItemModifiers } from "@/order/selections.ts"; -import { evaluateGuards, findOwnMeal, allPieces, ownPieces } from "@/order/guards.ts"; -import { deliveryStatus, formatDeliveryStatus } from "@/order/status.ts"; +import { evaluateGuards, allPieces, ownPieces } from "@/order/guards.ts"; +import { deliveryStatus, formatDeliveryStatus, ratingDetails } from "@/order/status.ts"; import { cancellationPending, formatMoney, @@ -37,14 +31,6 @@ function ok(t: string, structured?: Record): CallToolResult { function errResult(t: string): CallToolResult { return { content: text(t), isError: true }; } -function toCallToolResult(r: ToolResultLike): CallToolResult { - return { - content: r.content, - ...(r.structuredContent ? { structuredContent: r.structuredContent } : {}), - ...(r.isError ? { isError: true } : {}), - }; -} - // Markdown keeps dish images visible in clients that render tool text. function imageMd(item: { name: string; imageUrl?: string | null }): string { return item.imageUrl ? `\n ![${item.name}](${item.imageUrl})` : ""; @@ -118,16 +104,44 @@ function gateCtx(client: ForkableClient, session: SessionRecord): GateCtx { const WRITE_NOTE = "Returns a preview and confirmToken. Call again with the same arguments plus that token to send the change."; +// Reason codes used by Forkable's authenticated meal-rating UI. +const RATING_COMPLIMENTS = [ + "excellent_food", + "great_restaurant", + "organized_delivery", + "good_packaging", + "other", +] as const; +const RATING_ISSUES = [ + "food_quality", + "personal_preference", + "food_temp", + "portion_size", + "missing_ingredient_or_side", + "incorrect_meal", + "notes_not_followed", + "inaccurate_menu_info", + "bad_meal_suggestion", + "other", + "delivery_time", + "missing_meal", + "packaging", +] as const; + +function ratingFlag(value: boolean | null | undefined): string { + if (value == null) return "unchanged (not reported)"; + return value ? "yes" : "no"; +} + // `roles` is a feature-flag JSON scalar, not a member-role list. const ME_SELECTION = - "id firstName lastName fullName email phone active isGuest mfaEnabled validCreditCard " + + "id firstName lastName fullName email isGuest mfaEnabled validCreditCard " + "remainingLateOrdersMonthOf mealClubAutoOrder"; /** Read-only club policy. `allowanceMealLimit` is a boolean, not a count. */ const CLUB_POLICY_SEL = - "id name copay copayAllowance allowanceType allowanceMealLimit dailyAllowances " + - "allowLateMeals isLateRemovalEnabled deliveryDays hidePrices hiddenPriceLimit " + - "disableAutoOrder familyHub"; + "id name copayAllowance allowanceType allowanceMealLimit " + + "allowLateMeals isLateRemovalEnabled deliveryDays hidePrices disableAutoOrder"; interface ClubPolicy { id: number; @@ -141,7 +155,6 @@ interface ClubPolicy { hidePrices?: boolean; /** Club-level auto-order override. */ disableAutoOrder?: boolean; - familyHub?: boolean; } const WEEKDAY_NAMES = ["Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"] as const; @@ -177,58 +190,29 @@ function fmtClubPolicy(c: ClubPolicy): string { /** Add/replace refusal details. Requesting these fields from `removePiece` causes a server 503. */ const PIECE_WRITE_SEL = "errors errorDetails warningDetails"; -// Shared fields prevent duplicate selections. `orders.total` is company-wide cents and is omitted. +const RATING_SEL = "id level reasons comment forGuest allowRatingFollowUps attachment"; const DELIVERY_CORE = - "id state simpleState forDeliveryAt isReadOnly userConfirmed copayAmount availableMenuIds " + - "pastLateOrderDeadline canRequestChanges " + - // Preserve Forkable's direct billing fields without deriving coverage. - "allowanceType weeklyAllowance weeklyAllowanceAvailable " + - "forFamily forBuffet " + - // The list uses serviceWindow to distinguish lunch and dinner on the same date. - "deliveryWindow serviceWindow { baseTime name } " + - "club { id name allowanceMealLimit allowanceType familyHub isLateRemovalEnabled " + - "market { timezone currencySettings { currency } } }"; - -// Group and state are per-piece member fields; the order-level mealGroups roster is administrative. + "id state simpleState forDeliveryAt userConfirmed copayAmount availableMenuIds " + + "allowanceType weeklyAllowance weeklyAllowanceAvailable forBuffet " + + "deliveryWindow serviceWindow { baseTime name } club { id name market { timezone } }"; const PIECE_CORE = - "id itemId menuId userId name state instructions price selections autoOrder flowType group " + - "isConfirmed isLateSwappable isRemoval requestStatus isLateOrder"; - -// Each delivery selection appends its own `pieces` shape. + "id itemId menuId userId name price group isConfirmed isLateSwappable isRemoval requestStatus isLateOrder " + + `userRating { ${RATING_SEL} }`; const ORDER_CORE = - "id state isOverVenueCapacity lateOrdersRemaining lateGuestOrdersRemaining " + - "lateRemovalsRemaining changeRequestAllowed pastLateOrderDeadline hasChangeRequest " + - "menu { id name } replaces { id }"; - -/** Lean selection used by reads and write previews. */ + "id state menu { name } venue { name displayName } " + + "dropoffCompletedAt etaStatus { start end status shortTz trackingUrl }"; const DELIVERY_SEL = - `${DELIVERY_CORE} ` + - // ETA offsets provide a timezone fallback when the club has no IANA zone. - `orders { ${ORDER_CORE} pieces { ${PIECE_CORE} } ` + - "venue { id displayName familyHub } " + - // The list exposes tracking for delayed owned orders. - "dropoffCompletedAt etaStatus { start end status shortTz trackingUrl } } " + - // Receipt fields are returned directly; no coverage projection is derived. - "userReceipt { id due copayAmount clubCopay }"; - -/** Tracking detail fetched only by get_delivery_status. */ + `${DELIVERY_CORE} orders { ${ORDER_CORE} pieces { ${PIECE_CORE} } } ` + + "userReceipt { due clubCopay }"; const DELIVERY_DETAIL_SEL = - `${DELIVERY_CORE} reportMissingItemCutoff ` + - "address { street city postalCode formatted notes } " + - `orders { ${ORDER_CORE} pieces { ${PIECE_CORE} nonHiddenAttributes { label value } } ` + - "dropoffCompletedAt hasVenueLateOrdersRemaining " + - "replacementCutoffTs isNextStepsAble isReorderable " + - "etaStatus { start end shortTz status trackingUrl } " + - "venue { id name displayName capacity familyHub } " + - "dropoff { id route { courierId date } pickupWindowInfo { windowStart windowEnd } } } " + - "userReceipt { id due copayAmount clubCopay subtotal feesTotal fees { type fee } } " + - "myReportedIssues { id type resolution requestReOrder requestRefund requestGiftCard " + - "orders { id } pieces { id } }"; - + `${DELIVERY_CORE} reportMissingItemCutoff address { formatted notes } ` + + `orders { ${ORDER_CORE} replacementCutoffTs ` + + `pieces { ${PIECE_CORE} nonHiddenAttributes { label value } } } ` + + "userReceipt { due clubCopay }"; const MENU_SEL = "id name displayName disableSpecialInstructions " + - "sections { id name items { id menuId name description price imageUrl ingredientTags dietLevel modifierIds " + - "modifiers { id name display optionSetId min max required hidden options { id name price ingredientTags } } } } " + + "sections { items { id menuId name description price imageUrl dietLevel modifierIds " + + "modifiers { id name display optionSetId min max required hidden options { id name price } } } } " + "optionSets { id price }"; interface Me { @@ -484,7 +468,6 @@ function resolveOwnedSource( d: Delivery, userId: number, sourcePieceId?: string | number, - ambiguousMessage?: (count: number) => string, ): PieceTarget | undefined { const all = pieceTargets(d); if (sourcePieceId != null) { @@ -500,8 +483,7 @@ function resolveOwnedSource( const owned = all.filter(({ piece }) => piece.userId === userId); if (owned.length > 1) { throw new Error( - ambiguousMessage?.(owned.length) ?? - `You have ${owned.length} meals on delivery ${d.id}; pass sourcePieceId to choose which one to replace.`, + `You have ${owned.length} meals on delivery ${d.id}; pass sourcePieceId to choose which one to replace.`, ); } return owned[0]; @@ -556,7 +538,7 @@ function deliveryTag(d: Delivery): string { } export function fmtDelivery(d: Delivery, inFlight?: Set, userId?: number): string { - const own = findOwnMeal(d, userId)?.orders.flatMap((o) => o.pieces) ?? []; + const own = ownPieces(d, userId); const others = allPieces(d).length - own.length; // Group and state attach to each piece. const picked = own.length @@ -580,7 +562,7 @@ export function fmtDelivery(d: Delivery, inFlight?: Set, userId?: number /** The fields needed to identify a delivery and act on the effective user's meals. */ export function compactDelivery(d: Delivery, inFlight?: Set, userId?: number) { - const pieces = findOwnMeal(d, userId)?.orders.flatMap((o) => o.pieces) ?? []; + const pieces = ownPieces(d, userId); const { status: fulfillment, tracked } = fulfillmentSummary(d, userId); return { deliveryId: d.id, @@ -602,6 +584,7 @@ export function compactDelivery(d: Delivery, inFlight?: Set, userId?: nu group: p.group ?? null, isConfirmed: p.isConfirmed ?? null, cancellationPending: cancellationPending(p), + rating: ratingDetails(p.userRating), })), }; } @@ -852,17 +835,14 @@ export function registerAllTools(server: McpServer, writeGate: WriteGate): void }, async ({ deliveryId, limit }) => guard(async (client) => { - const [me, loaded] = await Promise.all([ - client.query<{ id: number }>("me", undefined, "id"), - loadDeliveries(client), - ]); - const d = findDelivery(loaded.deliveries, deliveryId); + const { deliveries, userId } = await loadDeliveries(client); + const d = findDelivery(deliveries, deliveryId); if (!d?.availableMenuIds?.length) return errResult(`Delivery ${deliveryId} not found or has no menus.`); const scores = (await client.query<{ menuId: number; itemId: number; score: number }[]>( "mealGenerationScores", - { deliveryId, menuIds: d.availableMenuIds, userId: me.id }, + { deliveryId, menuIds: d.availableMenuIds, userId }, "menuId itemId score", )) ?? []; const top = scores.toSorted((a, b) => b.score - a.score).slice(0, limit ?? 8); @@ -888,66 +868,6 @@ export function registerAllTools(server: McpServer, writeGate: WriteGate): void }), ); - server.registerTool( - "explain_pick", - { - title: "Explain the meal pick for a delivery", - description: - "Show where the selected meal ranks among Forkable's suggestions, with the top alternatives.", - inputSchema: z.object({ deliveryId: z.number().int() }), - annotations: { readOnlyHint: true, openWorldHint: true }, - }, - async ({ deliveryId }) => - guard(async (client) => { - const d = findDelivery((await loadDeliveries(client)).deliveries, deliveryId); - if (!d?.availableMenuIds?.length) - return errResult(`Delivery ${deliveryId} not found or has no menus.`); - const me = await client.query<{ id: number }>("me", undefined, "id"); - const scores = - (await client.query<{ menuId: number; itemId: number; score: number }[]>( - "mealGenerationScores", - { deliveryId, menuIds: d.availableMenuIds, userId: me.id }, - "menuId itemId score", - )) ?? []; - const ranked = scores.toSorted((x, y) => y.score - x.score); - const items = flattenItems(await loadMenus(client, d)); - const nameOf = (menuId: number, itemId: number) => - findItem(items, menuId, itemId)?.item.name ?? `item ${itemId}`; - // Match personalized scores only against the effective user's pieces. - const pieces = ownPieces(d, me.id); - if (!pieces.length) - return ok(`Delivery ${deliveryId} has no meal selected yet.`, { picked: null }); - - const picked = pieces.map((p) => { - const idx = ranked.findIndex((s) => s.menuId === p.menuId && s.itemId === p.itemId); - return { - itemId: p.itemId, - menuId: p.menuId, - name: p.name, - rank: idx >= 0 ? idx + 1 : null, - }; - }); - const top = ranked.slice(0, 5).map((s, index) => ({ - menuId: s.menuId, - itemId: s.itemId, - rank: index + 1, - name: nameOf(s.menuId, s.itemId), - })); - const lines = [ - "Your pick:", - ...picked.map((p) => - p.rank - ? ` ${p.name} — ranked #${p.rank} of ${ranked.length}` - : ` ${p.name} — not in Forkable's suggestions`, - ), - "", - "Top suggestions:", - ...top.map((suggestion) => ` ${suggestion.rank}. ${suggestion.name}`), - ]; - return ok(lines.join("\n"), { picked, top }); - }), - ); - server.registerTool( "get_delivery_status", { @@ -993,6 +913,7 @@ export function registerAllTools(server: McpServer, writeGate: WriteGate): void group: meal.group, isConfirmed: meal.isConfirmed, cancellationPending: meal.cancellationPending, + rating: meal.rating, })), orders: s.orders.map((order) => ({ orderId: order.orderId, @@ -1162,17 +1083,15 @@ export function registerAllTools(server: McpServer, writeGate: WriteGate): void guards, }; }; - return toCallToolResult( - await writeGate(gateCtx(client, session), { - tool: "set_meal", - argsHash: hashWriteArgs( - { ...a }, - { mode: "set", modifiers: [], instructions: "", autoConfirm: false }, - ), - confirmToken: a.confirmToken, - plan, - }), - ); + return writeGate(gateCtx(client, session), { + tool: "set_meal", + argsHash: hashWriteArgs( + { ...a }, + { mode: "set", modifiers: [], instructions: "", autoConfirm: false }, + ), + confirmToken: a.confirmToken, + plan, + }); }), ); @@ -1297,17 +1216,15 @@ export function registerAllTools(server: McpServer, writeGate: WriteGate): void guards, }; }; - return toCallToolResult( - await writeGate(gateCtx(client, session), { - tool: "set_meal_all", - argsHash: hashWriteArgs( - { ...a, deliveryIds: [...new Set(a.deliveryIds)] }, - { modifiers: [], instructions: "" }, - ), - confirmToken: a.confirmToken, - plan, - }), - ); + return writeGate(gateCtx(client, session), { + tool: "set_meal_all", + argsHash: hashWriteArgs( + { ...a, deliveryIds: [...new Set(a.deliveryIds)] }, + { modifiers: [], instructions: "" }, + ), + confirmToken: a.confirmToken, + plan, + }); }), ); @@ -1329,16 +1246,7 @@ export function registerAllTools(server: McpServer, writeGate: WriteGate): void const { deliveries, userId } = await loadDeliveries(client); const d = findDelivery(deliveries, a.deliveryId); if (!d) throw new Error(`Delivery ${a.deliveryId} not found.`); - const matches = pieceTargets(d).filter( - ({ piece }) => String(piece.id) === String(a.pieceId), - ); - if (matches.length !== 1) - throw new Error( - `Piece ${a.pieceId} was not found uniquely on delivery ${a.deliveryId}.`, - ); - const { order, piece } = matches[0]!; - if (piece.userId !== userId) - throw new Error(`Piece ${a.pieceId} is not verified as belonging to you.`); + const { order, piece } = resolveOwnedSource(d, userId, a.pieceId)!; return { op: "removePiece", selection: "errors", @@ -1348,27 +1256,40 @@ export function registerAllTools(server: McpServer, writeGate: WriteGate): void guards: [], }; }; - return toCallToolResult( - await writeGate(gateCtx(client, session), { - tool: "remove_meal", - argsHash: hashWriteArgs({ ...a }), - confirmToken: a.confirmToken, - plan, - }), - ); + return writeGate(gateCtx(client, session), { + tool: "remove_meal", + argsHash: hashWriteArgs({ ...a }), + confirmToken: a.confirmToken, + plan, + }); }), ); server.registerTool( - "skip_delivery", + "rate_meal", { - title: "Skip a delivery", + title: "Rate a meal", description: - "Decline a day by removing its single positively-owned meal. If you own several, remove " + - "them individually with remove_meal. " + + "Rate one of your meals from 1–5 or edit its feedback. Use pieceId from list_deliveries. " + + "Omitted feedback fields keep their existing values; empty reasons or comment clear them. " + + "Levels 4–5 accept compliments; levels 1–3 accept issues. " + + "Guest-meal ratings do not inform your future suggestions. " + WRITE_NOTE, inputSchema: z.object({ deliveryId: z.number().int(), + pieceId: z.union([z.string(), z.number()]), + level: z.number().int().min(1).max(5), + reasons: z + .array(z.enum([...RATING_COMPLIMENTS, ...RATING_ISSUES])) + .optional() + .describe("4–5: " + RATING_COMPLIMENTS.join(", ") + ". 1–3: " + RATING_ISSUES.join(", ")), + comment: z.string().optional(), + forGuest: z.boolean().optional(), + allowRatingFollowUps: z + .boolean() + .optional() + .describe("Allow Forkable to follow up on this rating; does not change account settings"), + from: dateArg().optional().describe("Search start (YYYY-MM-DD); defaults to 14 days ago"), confirmToken: z.string().optional(), }), annotations: { destructiveHint: true, idempotentHint: false, openWorldHint: true }, @@ -1376,37 +1297,70 @@ export function registerAllTools(server: McpServer, writeGate: WriteGate): void async (a) => guard(async (client, session) => { const plan = async (): Promise => { - const { deliveries, userId } = await loadDeliveries(client); - const d = findDelivery(deliveries, a.deliveryId); - if (!d) throw new Error(`Delivery ${a.deliveryId} not found.`); - if (userId == null) throw new Error("Forkable did not report your user id."); - const source = resolveOwnedSource( - d, - userId, - undefined, - (count) => - `You have ${count} verified meals on delivery ${d.id}; remove them individually with remove_meal and pieceId.`, + const range = deliveryRange(a.from ?? dateOffsetLocal(-14)); + const { deliveries, userId } = await loadDeliveries( + client, + range.from, + DELIVERY_SEL, + range.to, ); - if (!source) - throw new Error(`You have no verified meal on delivery ${a.deliveryId} to skip.`); - const { order, piece } = source; + const d = findDelivery(deliveries, a.deliveryId); + if (!d) + throw new Error( + `Delivery ${a.deliveryId} not found between ${range.from} and ${range.to}.`, + ); + if (d.forBuffet) + throw new Error( + "Buffet ratings are not supported; use Forkable to rate this delivery.", + ); + const { piece } = resolveOwnedSource(d, userId, a.pieceId)!; + const rating = piece.userRating; + if (rating?.id == null || rating.id === "") { + throw new Error( + `Forkable has not made a rating available for meal ${piece.id} on delivery ${d.id}.`, + ); + } + const allowed: readonly string[] = a.level >= 4 ? RATING_COMPLIMENTS : RATING_ISSUES; + if (a.reasons?.some((reason) => !allowed.includes(reason))) { + throw new Error(`A ${a.level}/5 rating accepts these reasons: ${allowed.join(", ")}.`); + } + const changedCategory = rating.level != null && rating.level >= 4 !== a.level >= 4; + const reasons = + a.reasons ?? + (changedCategory + ? (rating.reasons ?? []).filter((reason) => allowed.includes(reason)) + : (rating.reasons ?? [])); + const comment = a.comment ?? rating.comment ?? null; + const forGuest = a.forGuest ?? rating.forGuest; + const followUps = a.allowRatingFollowUps ?? rating.allowRatingFollowUps; return { - op: "removePiece", + op: "rateMeal", selection: "errors", - input: { orderId: order.id, pieceId: piece.id, myMeals: true }, - summary: `Skip delivery ${a.deliveryId} (${formatDay(d.forDeliveryAt)}) — removes ${piece.name ?? `piece ${piece.id}`}`, - deliveryIds: [a.deliveryId], - guards: [], + input: { + id: rating.id, + level: a.level, + reasons, + comment, + channel: "mc", + ...(forGuest != null ? { forGuest } : {}), + ...(followUps != null ? { allowRatingFollowUps: followUps } : {}), + ...(rating.attachment !== undefined ? { attachment: rating.attachment } : {}), + }, + summary: + `Rate ${piece.name ?? `meal ${piece.id}`} (piece ${piece.id}) on delivery ${d.id} ` + + `(${formatDay(d.forDeliveryAt)}) ${a.level}/5; reasons: ${reasons.join(", ") || "none"}; ` + + `comment: ${comment ? JSON.stringify(comment) : "none"}; guest meal: ${ratingFlag(forGuest)}; ` + + `allow follow-ups: ${ratingFlag(followUps)}${rating.attachment ? "; existing attachment kept" : ""}`, + deliveryIds: [d.id], + reconciliationRange: range, }; }; - return toCallToolResult( - await writeGate(gateCtx(client, session), { - tool: "skip_delivery", - argsHash: hashWriteArgs({ ...a }), - confirmToken: a.confirmToken, - plan, - }), - ); + return writeGate(gateCtx(client, session), { + tool: "rate_meal", + argsHash: hashWriteArgs({ ...a }), + confirmToken: a.confirmToken, + plan, + }); }), ); @@ -1438,14 +1392,12 @@ export function registerAllTools(server: McpServer, writeGate: WriteGate): void guards: [], }; }; - return toCallToolResult( - await writeGate(gateCtx(client, session), { - tool: "confirm_delivery", - argsHash: hashWriteArgs({ ...a }, { confirm: true }), - confirmToken: a.confirmToken, - plan, - }), - ); + return writeGate(gateCtx(client, session), { + tool: "confirm_delivery", + argsHash: hashWriteArgs({ ...a }, { confirm: true }), + confirmToken: a.confirmToken, + plan, + }); }), ); } diff --git a/src/write-gate.ts b/src/write-gate.ts index 4e9ffce..f904ff9 100644 --- a/src/write-gate.ts +++ b/src/write-gate.ts @@ -1,4 +1,5 @@ import { createHash, randomBytes } from "node:crypto"; +import type { CallToolResult } from "@modelcontextprotocol/server"; import { baseCodes, MutationError, MutationOutcomeUnknownError } from "@/net/errors.ts"; import { type Guard } from "@/order/guards.ts"; @@ -8,6 +9,7 @@ export interface ExecutableWritePlan { input: Record; summary: string; deliveryIds: number[]; + reconciliationRange?: { from: string; to: string }; } export interface WritePlan extends ExecutableWritePlan { @@ -24,12 +26,6 @@ export interface GateCtx { execute: (plan: ExecutableWritePlan) => Promise; } -export interface ToolResultLike { - content: { type: "text"; text: string }[]; - structuredContent?: Record; - isError?: boolean; -} - export interface WriteGateCall { tool: string; argsHash: string; @@ -37,7 +33,7 @@ export interface WriteGateCall { plan: () => Promise; } -export type WriteGate = (ctx: GateCtx, call: WriteGateCall) => Promise; +export type WriteGate = (ctx: GateCtx, call: WriteGateCall) => Promise; export interface WriteGateOptions { ttlMs?: number; @@ -147,6 +143,7 @@ export function createWriteGate(options: WriteGateOptions = {}): WriteGate { input: plan.input, summary: plan.summary, deliveryIds: plan.deliveryIds, + reconciliationRange: plan.reconciliationRange, }; pending.set(token, { ...binding, expiresAt, plan: structuredClone(executable) }); return { token, expiresAt }; @@ -170,7 +167,7 @@ export function createWriteGate(options: WriteGateOptions = {}): WriteGate { }; return async (ctx, call) => { - const blockedResult = (plan: WritePlan, note?: string): ToolResultLike => { + const blockedResult = (plan: WritePlan, note?: string): CallToolResult => { const blocking = blockers(plan.guards); return { isError: true, @@ -191,7 +188,7 @@ export function createWriteGate(options: WriteGateOptions = {}): WriteGate { const previewResult = async ( plan: WritePlan, confirmationError?: { reason: TakeFailure; message: string }, - ): Promise => { + ): Promise => { if (blockers(plan.guards).length) return blockedResult(plan, confirmationError?.message); const actor = await ctx.resolveActor(); @@ -277,6 +274,7 @@ export function createWriteGate(options: WriteGateOptions = {}): WriteGate { reconciliation: { tool: "list_deliveries", deliveryIds: plan.deliveryIds, + ...(plan.reconciliationRange ? { arguments: plan.reconciliationRange } : {}), }, }, }; diff --git a/tests/client.test.ts b/tests/client.test.ts index d414c31..be453c2 100644 --- a/tests/client.test.ts +++ b/tests/client.test.ts @@ -64,19 +64,6 @@ describe("mutation transport", () => { expect(calls).toBe(1); }); - test("gqlRaw uses mutation-safe retry behavior", async () => { - let calls = 0; - const c = client(async () => { - calls++; - throw new TypeError("connection closed after upload"); - }); - - expect( - await rejection(c.gqlRaw("mutation ($input: AddPieceInput!) { addPiece(input: $input) }")), - ).toBeInstanceOf(MutationOutcomeUnknownError); - expect(calls).toBe(1); - }); - test.each([302, 408, 503])("does not replay ambiguous HTTP %d", async (status) => { let calls = 0; let redirect: RequestRedirect | undefined; diff --git a/tests/order.test.ts b/tests/order.test.ts index ab85eca..7d1a66d 100644 --- a/tests/order.test.ts +++ b/tests/order.test.ts @@ -1,6 +1,6 @@ import { expect, test, describe } from "bun:test"; import { buildSelectionsHash, resolveItemModifiers } from "@/order/selections.ts"; -import { evaluateGuards, blockers, findOwnMeal, allPieces, ownPieces } from "@/order/guards.ts"; +import { evaluateGuards, blockers, ownedOrders, allPieces, ownPieces } from "@/order/guards.ts"; import { deliveryStatus, formatDeliveryStatus } from "@/order/status.ts"; import { formatMoney, @@ -8,8 +8,6 @@ import { formatDay, formatDateTime, weekdayOf, - parseFloating, - isPast, formatInstantLike, formatCountdown, groupSuffix, @@ -34,7 +32,7 @@ const protein: MenuModifier = { options: [ { id: 10, name: "Chicken", price: 0 }, { id: 11, name: "Steak", price: 3 }, - { id: 12, name: "Tofu", price: 0, ingredientTags: ["soy"] }, + { id: 12, name: "Tofu", price: 0 }, ], }; const extras: MenuModifier = { @@ -127,7 +125,7 @@ describe("buildSelectionsHash", () => { const soyFirst = { ...protein, options: [ - { id: 12, name: "Tofu", ingredientTags: ["soy"] }, + { id: 12, name: "Tofu" }, { id: 10, name: "Chicken" }, ], }; @@ -176,11 +174,9 @@ describe("buildSelectionsHash", () => { expect(r.violations.some((v) => v.code === "duplicate_modifier")).toBe(true); }); - test("explicit empty single choices do not preserve or default", () => { - const previous = { "16": [11], "18": [31] }; + test("explicit empty single choices do not default", () => { const r = buildSelectionsHash({ item, - previous, choices: [ { modifier: 16, options: [] }, { modifier: 18, options: [] }, @@ -206,17 +202,6 @@ describe("buildSelectionsHash", () => { const r = buildSelectionsHash({ item, choices: [{ modifier: 16, options: ["Lobster"] }] }); expect(r.violations.some((v) => v.code === "unknown_option")).toBe(true); }); - - test("round-trips an existing piece's selections byte-for-byte", () => { - const stored = { "16": [11], "17": [20], "18": [-1] }; - const rebuilt = buildSelectionsHash({ item, previous: stored }).selectionsHash; - expect(rebuilt).toEqual(stored); - }); - - test("round-trips a multi with several options and an unset required-less single", () => { - const stored = { "16": [10], "17": [20, 21], "18": [-1] }; - expect(buildSelectionsHash({ item, previous: stored }).selectionsHash).toEqual(stored); - }); }); describe("evaluateGuards", () => { @@ -294,32 +279,6 @@ describe("weekdayOf", () => { }); }); -describe("parseFloating", () => { - test("honors a real UTC offset as a true instant", () => { - expect(parseFloating(CUTOFF)?.toISOString()).toBe("2026-08-10T18:45:00.000Z"); - }); - - test("treats a mislabelled `Z` as local wall-clock time", () => { - const at = parseFloating(FOR_DELIVERY)!; - expect(at.getFullYear()).toBe(2026); - expect(at.getMonth()).toBe(7); // August - expect(at.getDate()).toBe(11); - expect(at.getHours()).toBe(12); // 12:01 local, NOT 5:01 after a UTC shift - expect(at.getMinutes()).toBe(1); - }); - - test("parses a date-only value as local midnight, not UTC", () => { - const at = parseFloating("2026-08-11")!; - expect(at.getDate()).toBe(11); // `new Date("2026-08-11")` alone lands on the 10th west of UTC - expect(at.getHours()).toBe(0); - }); - - test("returns undefined for missing/invalid input", () => { - expect(parseFloating(undefined)).toBeUndefined(); - expect(parseFloating("not-a-date")).toBeUndefined(); - }); -}); - describe("formatDateTime", () => { // Rendering uses the timestamp's named wall clock without host-zone conversion. test("shows the cutoff as the dashboard does", () => { @@ -341,19 +300,6 @@ describe("formatDateTime", () => { }); }); -describe("isPast", () => { - const now = new Date("2026-08-11T01:09:00.000Z"); // Mon Aug 10, 6:09 PM PDT - test("the Aug 11 delivery's cutoff has already passed", () => { - expect(isPast(CUTOFF, now)).toBe(true); - }); - test("a later cutoff has not", () => { - expect(isPast("2026-08-11T11:45:00-07:00", now)).toBe(false); - }); - test("undefined when there's nothing to compare", () => { - expect(isPast(undefined, now)).toBeUndefined(); - }); -}); - describe("formatDate", () => { test("keeps the calendar date the API named", () => { expect(formatDate(FOR_DELIVERY)).toBe("2026-08-11"); @@ -370,58 +316,39 @@ const myPiece = { menuId: 3, name: "Nebula Noodles", price: 18.99, - autoOrder: true, }; -/** Four venue orders with the user's meal last. */ -const fourOrders: Delivery = { - id: 1234199, - availableMenuIds: [1, 2, 3, 4], - orders: [ - { id: 1, menu: { id: 1, name: "Fixture Diner" }, lateOrdersRemaining: 0 }, - { id: 2, menu: { id: 2, name: "Taqueria Los Altos" }, lateOrdersRemaining: 6 }, - { id: 3, menu: { id: 3, name: "Kitava" }, lateOrdersRemaining: 6 }, - { - id: 4, - menu: { id: 4, name: "Placeholder Kitchen" }, - lateOrdersRemaining: 6, - pieces: [myPiece], - }, - ], -}; - -describe("findOwnMeal / allPieces", () => { - test("finds the order holding your pieces, not orders[0]", () => { - const own = findOwnMeal(fourOrders); - expect(own?.order.id).toBe(4); - expect(own?.pieces.length).toBe(1); - expect(own?.ambiguous).toBe(false); - }); - - test("undefined when no order carries pieces", () => { - expect(findOwnMeal({ id: 1, orders: [{ id: 1 }, { id: 2 }] })).toBeUndefined(); - expect(findOwnMeal({ id: 1 })).toBeUndefined(); - }); - - test("several orders with pieces → first, flagged ambiguous", () => { - const d: Delivery = { - id: 1, - orders: [ - { id: 1, pieces: [myPiece] }, - { id: 2, pieces: [myPiece] }, - ], - }; - expect(findOwnMeal(d)?.order.id).toBe(1); - expect(findOwnMeal(d)?.ambiguous).toBe(true); - }); +describe("ownedOrders / allPieces", () => { + const ME = 501; + const shared: Delivery = { + id: 1, + orders: [ + { id: 1, pieces: [{ ...myPiece, id: "theirs", userId: 999 }] }, + { id: 2, pieces: [{ ...myPiece, id: "mine", userId: ME }] }, + { id: 3, pieces: [{ ...myPiece, id: "unknown" }] }, + { id: 4, pieces: [{ ...myPiece, id: "also-mine", userId: ME }] }, + ], + }; - test("allPieces flattens every venue order (guest picks included)", () => { - const d: Delivery = { - id: 1, - orders: [{ id: 1, pieces: [myPiece] }, { id: 2 }, { id: 3, pieces: [myPiece, myPiece] }], - }; - expect(allPieces(d).length).toBe(3); - expect(allPieces({ id: 1, orders: [] })).toEqual([]); + test("collects all positively owned orders and pieces", () => { + expect(ownedOrders(shared, ME).map(({ order }) => order.id)).toEqual([2, 4]); + expect(ownPieces(shared, ME).map((piece) => piece.id)).toEqual(["mine", "also-mine"]); + expect( + ownPieces({ ...shared, orders: shared.orders!.toReversed() }, ME).map((piece) => piece.id), + ).toEqual(["also-mine", "mine"]); + expect(ownedOrders({ id: 1 }, ME)).toEqual([]); + expect(allPieces(shared)).toHaveLength(4); + }); + + test("missing identity never claims another member's meal or rating", () => { + expect(ownedOrders(shared)).toEqual([]); + expect(ownPieces(shared)).toEqual([]); + expect(compactDelivery(shared).meals).toEqual([]); + expect(deliveryStatus(shared).meal).toEqual([]); + const output = fmtDelivery(shared); + expect(output).toContain("nothing selected"); + expect(output).toContain("+4 other meals"); + expect(output).not.toContain(myPiece.name); }); }); @@ -466,19 +393,17 @@ const DELIVERED: Delivery = { state: "grace_period", simpleState: "delivered", forDeliveryAt: FOR_DELIVERY, - isReadOnly: true, - pastLateOrderDeadline: true, deliveryWindow: ["11:45", "12:15"], serviceWindow: { baseTime: "12:00:00", name: "lunch" }, reportMissingItemCutoff: "2026-08-11T20:00:00.000Z", address: { formatted: "350 Rhode Island St, San Francisco, CA", notes: "Gate code #1234" }, copayAmount: 20, orders: [ - { id: 1, menu: { id: 1, name: "Fixture Diner" } }, + { id: 1, menu: { name: "Fixture Diner" } }, { id: 4, - menu: { id: 4, name: "Placeholder Kitchen" }, - venue: { id: 3, displayName: "Placeholder Kitchen" }, + menu: { name: "Placeholder Kitchen" }, + venue: { displayName: "Placeholder Kitchen" }, dropoffCompletedAt: "2026-08-11T18:41:44.000Z", etaStatus: { start: ETA_START, @@ -500,9 +425,6 @@ describe("deliveryStatus", () => { { orderId: 4, venue: "Placeholder Kitchen", - pieceIds: ["p1"], - state: null, - etaStatus: "delivered", fulfillment: "delivered", dropoffCompletedAt: "2026-08-11T18:41:44.000Z", etaStart: ETA_START, @@ -514,7 +436,6 @@ describe("deliveryStatus", () => { expect(s.deliveryWindow).toEqual(["11:45", "12:15"]); expect(s.service).toBe("lunch, base 12:00"); expect(s.meal[0]?.venue).toBe("Placeholder Kitchen"); - expect(s.meal[0]?.autoOrder).toBe(true); }); test("a pre-dispatch day degrades to nulls, not throws", () => { @@ -578,7 +499,7 @@ describe("deliveryStatus", () => { { id: 10, state: "ready", - venue: { id: 10, displayName: "First Cafe" }, + venue: { displayName: "First Cafe" }, etaStatus: { status: "ontime", start: ETA_START, @@ -592,7 +513,7 @@ describe("deliveryStatus", () => { { id: 20, state: "ready", - venue: { id: 20, displayName: "Second Cafe" }, + venue: { displayName: "Second Cafe" }, etaStatus: { status: "delayed", start: ETA_START, @@ -610,14 +531,13 @@ describe("deliveryStatus", () => { expect(status.fulfillment).toBe("partially delivered"); expect(status.delayed).toBe(true); expect( - status.orders.map(({ orderId, pieceIds, trackingUrl }) => ({ + status.orders.map(({ orderId, trackingUrl }) => ({ orderId, - pieceIds, trackingUrl, })), ).toEqual([ - { orderId: 10, pieceIds: ["first"], trackingUrl: "https://track.test/first" }, - { orderId: 20, pieceIds: ["second"], trackingUrl: "https://track.test/second" }, + { orderId: 10, trackingUrl: "https://track.test/first" }, + { orderId: 20, trackingUrl: "https://track.test/second" }, ]); expect(status.meal.map(({ pieceId, orderId }) => ({ pieceId, orderId }))).toEqual([ { pieceId: "first", orderId: 10 }, @@ -697,8 +617,6 @@ describe("formatDeliveryStatus", () => { // 20:00Z is 1 PM Pacific on this date. expect(s.reportMissingItemCutoff).toBe("Tue 2026-08-11 1:00 PM PT"); expect(formatDeliveryStatus(s)).toContain("Report by : Tue 2026-08-11 1:00 PM PT"); - // Preserve the raw instant alongside the formatted value. - expect(s.reportMissingItemCutoffRaw).toBe("2026-08-11T20:00:00.000Z"); expect(formatDeliveryStatus(s)).not.toContain("20:00"); }); @@ -723,7 +641,7 @@ describe("meal groups", () => { orders: [ { id: 1, - venue: { id: 1, displayName: "Stub Street Cafe" }, + venue: { displayName: "Stub Street Cafe" }, pieces: [{ ...myPiece, userId: ME, name: "Comet Curry", group: "A1" }], }, ], @@ -750,7 +668,7 @@ describe("meal groups", () => { }, { id: 2, - venue: { id: 2, displayName: "Mock Market Kitchen" }, + venue: { displayName: "Mock Market Kitchen" }, pieces: [{ ...myPiece, id: "c", userId: ME, name: "Quasar Bowl", group: "A5" }], }, ], @@ -875,106 +793,6 @@ describe("meal groups", () => { }); }); -describe("findOwnMeal with a guest order", () => { - const ME = 501; - const guestFirst: Delivery = { - id: 9, - orders: [ - { - id: 1, - menu: { id: 1 }, - pieces: [{ ...myPiece, id: "guest-1", userId: 999, name: "Guest burrito" }], - }, - { id: 2, menu: { id: 2 }, pieces: [{ ...myPiece, id: "mine-1", userId: ME }] }, - ], - }; - - test("without a userId it picks the wrong piece — a guest's — and says so", () => { - const own = findOwnMeal(guestFirst); - expect(own?.pieces[0]?.id).toBe("guest-1"); - expect(own?.ambiguous).toBe(true); - }); - - test("with a userId it resolves the right piece and is unambiguous", () => { - const own = findOwnMeal(guestFirst, ME); - expect(own?.pieces[0]?.id).toBe("mine-1"); - expect(own?.order.id).toBe(2); - expect(own?.ambiguous).toBe(false); - }); - - test("order position cannot change the answer once userId is supplied", () => { - const reversed: Delivery = { ...guestFirst, orders: (guestFirst.orders ?? []).toReversed() }; - expect(findOwnMeal(reversed, ME)?.pieces[0]?.id).toBe("mine-1"); - }); - - test("pieces with no userId are not claimed for anyone", () => { - // An unattributed piece does not satisfy an identified lookup. - expect(findOwnMeal(fourOrders, ME)).toBeUndefined(); - // An unidentified display lookup remains explicitly unattributed. - expect(findOwnMeal(fourOrders)?.order.id).toBe(4); - }); -}); - -describe("findOwnMeal across venues", () => { - const ME = 501; - const twoVenues: Delivery = { - id: 9, - orders: [ - { id: 1, menu: { id: 1 }, pieces: [{ ...myPiece, id: "a", userId: ME }] }, - { id: 2, menu: { id: 2 }, pieces: [{ ...myPiece, id: "b", userId: 999 }] }, - { id: 3, menu: { id: 3 }, pieces: [{ ...myPiece, id: "c", userId: ME }] }, - ], - }; - - test("collects every order the member holds a piece on", () => { - const own = findOwnMeal(twoVenues, ME); - expect(own?.orders.map((o) => o.order.id)).toEqual([1, 3]); - expect(own?.ambiguous).toBe(true); - expect(own?.order.id).toBe(1); - }); - - test("a guest's piece is never collected", () => { - expect(ownPieces(twoVenues, ME).map((p) => p.id)).toEqual(["a", "c"]); - }); - - test("one venue is not ambiguous", () => { - const one: Delivery = { id: 9, orders: [{ id: 1, pieces: [{ ...myPiece, userId: ME }] }] }; - expect(findOwnMeal(one, ME)?.ambiguous).toBe(false); - }); -}); - -describe("multi-venue writes target the right meal", () => { - const ME = 501; - // Two owned meals exercise per-venue identity. - const twoVenues: Delivery = { - id: 9, - availableMenuIds: [1, 2], - orders: [ - { id: 1, menu: { id: 1 }, pieces: [{ ...myPiece, id: "at-venue-1", menuId: 1, userId: ME }] }, - { id: 2, menu: { id: 2 }, pieces: [{ ...myPiece, id: "at-venue-2", menuId: 2, userId: ME }] }, - ], - }; - - test("findOwnMeal exposes the per-venue split needed to pick the right piece", () => { - const own = findOwnMeal(twoVenues, ME)!; - // Menu 2 resolves to its own piece rather than the first order. - expect(own.order.menu?.id).toBe(1); - const target = own.orders.find((x) => x.order.menu?.id === 2); - expect(target?.pieces[0]?.id).toBe("at-venue-2"); - expect(own.byIdentity).toBe(true); - }); - - test("someone else's meal is never returned as the member's", () => { - const theirsOnly: Delivery = { - id: 9, - orders: [{ id: 1, pieces: [{ ...myPiece, userId: 999 }] }], - }; - expect(findOwnMeal(theirsOnly, ME)).toBeUndefined(); - // Missing identity is reflected in the attribution flag. - expect(findOwnMeal(theirsOnly)?.byIdentity).toBe(false); - }); -}); - describe("per-piece state badges", () => { const ME = 501; const deliveryWith = (piece: object): Delivery => ({ @@ -983,7 +801,7 @@ describe("per-piece state badges", () => { orders: [ { id: 1, - venue: { id: 1, displayName: "Stub Street Cafe" }, + venue: { displayName: "Stub Street Cafe" }, pieces: [{ ...myPiece, userId: ME, name: "Comet Curry", ...piece }], }, ], @@ -1228,7 +1046,6 @@ describe("formatCountdown / the replacement clock", () => { }; const s = deliveryStatus(d, ME, NOW); expect(s.replacementCountdown).toBe("2h 14m"); - expect(s.replacementCutoffRaw).toBe("2026-08-12T20:14:00Z"); expect(formatDeliveryStatus(s)).toContain( "Re-pick by : 2h 14m left — the restaurant cancelled", ); @@ -1299,19 +1116,16 @@ describe("formatCountdown / the replacement clock", () => { }; const s = deliveryStatus(d, ME, NOW); expect(s.replacementCountdown).toBeNull(); - // Preserve the raw cutoff after the countdown expires. - expect(s.replacementCutoffRaw).toBe("2026-08-12T17:00:00Z"); expect(formatDeliveryStatus(s)).not.toContain("Re-pick"); }); - test("no replacement means no line, and the raw value stays null", () => { + test("no replacement means no countdown or line", () => { const s = deliveryStatus( { id: 8, forDeliveryAt: FOR_DELIVERY, orders: [{ id: 1, pieces: [{ ...myPiece }] }] }, undefined, NOW, ); expect(s.replacementCountdown).toBeNull(); - expect(s.replacementCutoffRaw).toBeNull(); expect(formatDeliveryStatus(s)).not.toContain("Re-pick"); }); }); @@ -1328,21 +1142,21 @@ describe("money reaches the rendered line", () => { }); test("the list line shows the direct reported due", () => { - const line = fmtDelivery(withReceipt({ id: 1, clubCopay: 20, due: 4.5 }), undefined, ME); + const line = fmtDelivery(withReceipt({ clubCopay: 20, due: 4.5 }), undefined, ME); expect(line).toContain("reported due $4.50"); expect(line).not.toContain("company covers"); expect(line).not.toContain("you pay"); }); test("a reported zero due is preserved", () => { - const line = fmtDelivery(withReceipt({ id: 1, clubCopay: 20, due: 0 }), undefined, ME); + const line = fmtDelivery(withReceipt({ clubCopay: 20, due: 0 }), undefined, ME); expect(line).toContain("reported due $0.00"); expect(line).not.toContain("company covers"); expect(line).not.toContain("you pay"); }); test("the status view exposes direct billing values as cents", () => { - const s = deliveryStatus(withReceipt({ id: 1, clubCopay: 20, due: 4.5 }), ME); + const s = deliveryStatus(withReceipt({ clubCopay: 20, due: 4.5 }), ME); expect(s.billing).toEqual({ reportedDueCents: 450, allowanceType: "daily", @@ -1364,12 +1178,8 @@ describe("money reaches the rendered line", () => { }); test("the compact read omits server policy signals", () => { - const compact = compactDelivery({ - id: 4, - isReadOnly: true, - pastLateOrderDeadline: true, - canRequestChanges: false, - }); + const wire = { id: 4, isReadOnly: true, pastLateOrderDeadline: true, canRequestChanges: false }; + const compact = compactDelivery(wire); expect(compact).not.toHaveProperty("isReadOnly"); expect(compact).not.toHaveProperty("pastLateOrderDeadline"); expect(compact).not.toHaveProperty("canRequestChanges"); @@ -1388,14 +1198,14 @@ describe("a delivery carrying another member's order", () => { orders: [ { id: 1, - menu: { id: 1, name: "Their Venue" }, + menu: { name: "Their Venue" }, pieces: [{ id: "theirs", itemId: 1, menuId: 1, userId: THEM, name: "Their Burrito" }], dropoffCompletedAt: "2026-08-11T18:41:44.000Z", etaStatus: { start: "2026-08-11T11:35:00-07:00", status: "delivered", shortTz: "PT" }, }, { id: 2, - menu: { id: 2, name: "My Venue" }, + menu: { name: "My Venue" }, pieces: [{ id: "mine", itemId: 2, menuId: 2, userId: ME, name: "My Noodles" }], }, ], diff --git a/tests/tools-rating.test.ts b/tests/tools-rating.test.ts new file mode 100644 index 0000000..33bb4da --- /dev/null +++ b/tests/tools-rating.test.ts @@ -0,0 +1,343 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import type { CallToolResult, McpServer } from "@modelcontextprotocol/server"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { patchSession } from "@/auth/session.ts"; +import { ENDPOINT } from "@/net/endpoints.ts"; +import type { Delivery, Piece } from "@/order/types.ts"; +import { addDaysLocal, deliveryRange, registerAllTools } from "@/tools.ts"; +import { createWriteGate } from "@/write-gate.ts"; + +type Handler = (args: Record) => Promise; +const USER_ID = 42; +const ratingArgs = { deliveryId: 1, pieceId: "mine", level: 5, from: "2026-08-01" }; +const structured = (result: CallToolResult) => + (result.structuredContent ?? {}) as Record; +const textOf = (result: CallToolResult) => + result.content.flatMap((block) => (block.type === "text" ? [block.text] : [])).join("\n"); +const tokenOf = (result: CallToolResult) => { + const token = structured(result).confirmToken; + if (typeof token !== "string") throw new Error("No confirmation token"); + return token; +}; + +describe("meal ratings", () => { + let home: string; + let originalHome: string | undefined; + let originalFetch: typeof fetch; + let piece: Piece; + let delivery: Delivery; + let handlers: Map; + let queries: string[]; + let mutations: { query: string; variables: { input: Record } }[]; + let mutationResponse: Record | Error; + let reportedUserId: number | null; + + beforeEach(async () => { + originalHome = process.env.FORKABLE_MCP_HOME; + home = mkdtempSync(join(tmpdir(), "forkable-ratings-")); + process.env.FORKABLE_MCP_HOME = home; + await patchSession({ cookie: "_easyorder_session=test", csrf: "test-csrf" }); + piece = { + id: "mine", + itemId: 20, + menuId: 10, + userId: USER_ID, + name: "Lunch bowl", + userRating: { id: 500, level: null, reasons: [], comment: null }, + }; + delivery = { + id: 1, + forDeliveryAt: "2026-08-28T12:00:00Z", + orders: [{ id: 100, pieces: [piece] }], + }; + queries = []; + mutations = []; + mutationResponse = { data: { rateMeal: { errors: [] } } }; + reportedUserId = USER_ID; + originalFetch = globalThis.fetch; + globalThis.fetch = (async (url: string, init?: RequestInit) => { + expect(url).toBe(ENDPOINT); + const body = JSON.parse(init?.body as string); + if (body.query.startsWith("mutation")) { + mutations.push(body); + expect(init?.redirect).toBe("manual"); + const headers = new Headers(init?.headers); + expect(headers.get("cookie")).toBe("_easyorder_session=test"); + expect(headers.get("x-csrf-token")).toBe("test-csrf"); + if (mutationResponse instanceof Error) throw mutationResponse; + return Response.json(mutationResponse); + } + queries.push(body.query); + if (body.query.includes("myDeliveries")) { + return Response.json({ data: { myDeliveries: [delivery], me: { id: reportedUserId } } }); + } + if (body.query.includes("myInProgressDeliveryIds")) { + return Response.json({ data: { myInProgressDeliveryIds: [] } }); + } + if (body.query === "{ me { id } }") + return Response.json({ data: { me: { id: reportedUserId } } }); + throw new Error(`Unexpected query: ${body.query}`); + }) as typeof fetch; + handlers = new Map(); + const server = { + registerTool( + name: string, + definition: { inputSchema?: { parse: (args: unknown) => unknown } }, + handler: Handler, + ) { + handlers.set(name, async (args) => + handler((definition.inputSchema?.parse(args) ?? args) as Record), + ); + }, + } as unknown as McpServer; + registerAllTools(server, createWriteGate()); + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + if (originalHome === undefined) delete process.env.FORKABLE_MCP_HOME; + else process.env.FORKABLE_MCP_HOME = originalHome; + rmSync(home, { recursive: true, force: true }); + }); + + const callRating = (overrides: Record = {}) => + handlers.get("rate_meal")!({ ...ratingArgs, ...overrides }); + + test("previews an initial score and sends exactly one authenticated stored mutation", async () => { + const preview = await callRating(); + expect(structured(preview).mode).toBe("preview"); + expect(textOf(preview)).toContain("Lunch bowl (piece mine)"); + expect(textOf(preview)).toContain("5/5"); + expect(textOf(preview)).toContain("allow follow-ups: unchanged (not reported)"); + expect(textOf(preview)).not.toContain("500"); + expect(mutations).toEqual([]); + expect(queries.filter((query) => query.includes("myDeliveries"))).toHaveLength(1); + // Later state must not silently change the confirmed request. + piece.userRating!.comment = "Changed after preview"; + const result = await callRating({ confirmToken: tokenOf(preview) }); + expect(structured(result).mode).toBe("executed"); + expect(mutations).toEqual([ + { + query: "mutation ($input: RateMealInput!) { rateMeal(input: $input) { errors } }", + variables: { input: { id: 500, level: 5, reasons: [], comment: null, channel: "mc" } }, + }, + ]); + expect(queries.filter((query) => query.includes("myDeliveries"))).toHaveLength(1); + const reused = await callRating({ confirmToken: tokenOf(preview) }); + expect(structured(reused).confirmationError.reason).toBe("unknown"); + expect(mutations).toHaveLength(1); + }); + + test("preserves omitted feedback and attachments without changing account settings", async () => { + piece.userRating = { + id: 500, + level: 4, + reasons: ["excellent_food"], + comment: "Keep this", + forGuest: true, + allowRatingFollowUps: false, + attachment: "https://example.com/existing.jpg", + }; + const preview = await callRating(); + expect(textOf(preview)).toContain('comment: "Keep this"'); + expect(textOf(preview)).toContain( + "guest meal: yes; allow follow-ups: no; existing attachment kept", + ); + await callRating({ confirmToken: tokenOf(preview) }); + expect(mutations[0]!.variables.input).toEqual({ ...piece.userRating, level: 5, channel: "mc" }); + expect(mutations).toHaveLength(1); + }); + + test("explicit empty feedback and false preferences replace stored values", async () => { + piece.userRating = { + id: 500, + level: 4, + reasons: ["excellent_food"], + comment: "Old comment", + forGuest: true, + allowRatingFollowUps: true, + }; + const edit = { reasons: [], comment: "", forGuest: false, allowRatingFollowUps: false }; + const preview = await callRating(edit); + await callRating({ ...edit, confirmToken: tokenOf(preview) }); + expect(mutations[0]!.variables.input).toEqual({ id: 500, level: 5, channel: "mc", ...edit }); + }); + + test.each([ + { previous: 5, next: 2, reasons: ["excellent_food", "other"], kept: ["other"] }, + { previous: 2, next: 4, reasons: ["food_quality", "other"], kept: ["other"] }, + ])( + "filters incompatible stored reasons when changing from $previous to $next", + async ({ previous, next, reasons, kept }) => { + piece.userRating = { + id: 500, + level: previous, + reasons: [...reasons], + comment: "Preserve written feedback", + }; + const preview = await callRating({ level: next }); + await callRating({ level: next, confirmToken: tokenOf(preview) }); + expect(mutations[0]!.variables.input.reasons).toEqual(kept); + expect(mutations[0]!.variables.input.comment).toBe("Preserve written feedback"); + }, + ); + + test.each([ + { level: 5, reasons: ["food_quality"] }, + { level: 2, reasons: ["excellent_food"] }, + ])("refuses incompatible explicit reasons for $level without a token", async (edit) => { + const result = await callRating(edit); + expect(result.isError).toBe(true); + expect(structured(result).confirmToken).toBeUndefined(); + expect(mutations).toEqual([]); + }); + + test.each([ + { level: 0 }, + { level: 6 }, + { level: 2.5 }, + { reasons: ["invented_reason"] }, + { from: "2026-02-30" }, + ])("rejects invalid input %j at the tool schema", async (edit) => { + await expect(callRating(edit)).rejects.toThrow(); + expect(queries).toEqual([]); + expect(mutations).toEqual([]); + }); + + test.each([ + "missing", + "duplicate", + "other_owner", + "unknown_owner", + "unavailable", + "empty_id", + "missing_actor", + "buffet", + "wrong_delivery", + ])("does not preview an unsafe or unavailable target: %s", async (scenario) => { + if (scenario === "missing") delivery.orders![0]!.pieces = []; + if (scenario === "duplicate") delivery.orders![0]!.pieces!.push({ ...piece }); + if (scenario === "other_owner") piece.userId = 99; + if (scenario === "unknown_owner") delete piece.userId; + if (scenario === "unavailable") piece.userRating = null; + if (scenario === "empty_id") piece.userRating!.id = ""; + if (scenario === "missing_actor") reportedUserId = null; + if (scenario === "buffet") delivery.forBuffet = true; + if (scenario === "wrong_delivery") delivery.id = 2; + const result = await callRating(); + expect(result.isError).toBe(true); + expect(structured(result).confirmToken).toBeUndefined(); + expect(mutations).toEqual([]); + }); + + test("targets the requested piece across several owned and unowned venue orders", async () => { + delivery.orders!.unshift({ + id: 99, + pieces: [{ ...piece, id: "other", userId: 99, userRating: { id: 900 } }], + }); + delivery.orders!.push({ + id: 101, + pieces: [{ ...piece, id: "also-mine", userRating: { id: 501 } }], + }); + const preview = await callRating(); + await callRating({ confirmToken: tokenOf(preview) }); + expect(mutations[0]!.variables.input.id).toBe(500); + }); + + test("binds changed score and feedback to confirmation", async () => { + const preview = await callRating(); + const result = await callRating({ + level: 4, + comment: "Different", + confirmToken: tokenOf(preview), + }); + expect(structured(result).confirmationError.reason).toBe("args_changed"); + expect(mutations).toEqual([]); + }); + + test("searches recent history by default and accepts an older explicit window", async () => { + await callRating({ from: undefined }); + const range = deliveryRange(addDaysLocal(new Date().toLocaleDateString("en-CA"), -14)); + expect(queries[0]).toContain(`from: "${range.from}", to: "${range.to}"`); + await callRating(); + expect(queries.find((query) => query.includes('from: "2026-08-01"'))).toBeDefined(); + }); + + test("returns historical reconciliation without replaying an uncertain mutation", async () => { + const preview = await callRating(); + mutationResponse = new TypeError("Connection closed after upload"); + const result = await callRating({ confirmToken: tokenOf(preview) }); + expect(structured(result)).toMatchObject({ + mode: "outcome_unknown", + retrySafe: false, + reconciliation: { + tool: "list_deliveries", + deliveryIds: [1], + arguments: deliveryRange(ratingArgs.from), + }, + }); + expect(mutations).toHaveLength(1); + const refreshed = await handlers.get("list_deliveries")!( + structured(result).reconciliation.arguments, + ); + expect(structured(refreshed).deliveries[0].meals[0].rating.level).toBeNull(); + }); + + test("surfaces a definite Forkable refusal without replaying", async () => { + mutationResponse = { data: { rateMeal: { errors: ["rating_locked"] } } }; + const preview = await callRating(); + const result = await callRating({ confirmToken: tokenOf(preview) }); + expect(structured(result)).toMatchObject({ mode: "rejected", reasons: ["rating_locked"] }); + expect(mutations).toHaveLength(1); + }); + + test("list and status expose only owned feedback, distinguishing unavailable from unrated", async () => { + delivery.orders![0]!.pieces!.push({ ...piece, id: "not-available", userRating: null }); + delivery.orders![0]!.pieces!.push({ + ...piece, + id: "theirs", + userId: 99, + userRating: { id: 900, level: 1, comment: "Private feedback" }, + }); + const rating = { + level: null, + reasons: [], + comment: null, + forGuest: null, + allowRatingFollowUps: null, + }; + const tools = ["list_deliveries", "get_delivery_status"]; + const results = await Promise.all( + tools.map((tool) => handlers.get(tool)!({ deliveryId: 1, from: ratingArgs.from })), + ); + for (const [index, result] of results.entries()) { + const tool = tools[index]; + const data = structured(result); + const meals = tool === "list_deliveries" ? data.deliveries[0].meals : data.status.meals; + expect(meals).toHaveLength(2); + expect(meals.map((meal: { rating: unknown }) => meal.rating)).toEqual([rating, null]); + expect(JSON.stringify(result)).not.toContain("Private feedback"); + expect(meals[0].rating).not.toHaveProperty("id"); + expect(meals[0].rating).not.toHaveProperty("channel"); + } + piece.userRating = { + id: 500, + level: 2, + reasons: ["food_temp"], + comment: "Cold", + forGuest: false, + allowRatingFollowUps: true, + }; + const result = await handlers.get("get_delivery_status")!({ deliveryId: 1 }); + expect(structured(result).status.meals[0].rating).toEqual({ + ...rating, + level: 2, + reasons: ["food_temp"], + comment: "Cold", + forGuest: false, + allowRatingFollowUps: true, + }); + }); +}); diff --git a/tests/tools-read.test.ts b/tests/tools-read.test.ts index c3506e9..2c15d3d 100644 --- a/tests/tools-read.test.ts +++ b/tests/tools-read.test.ts @@ -22,9 +22,6 @@ const delivery: Delivery = { state: "scheduled", userConfirmed: true, availableMenuIds: [MENU_ID], - isReadOnly: true, - pastLateOrderDeadline: true, - canRequestChanges: false, allowanceType: "weekly", copayAmount: 20, weeklyAllowance: 100, @@ -33,11 +30,11 @@ const delivery: Delivery = { deliveryWindow: ["11:30 AM", "12:30 PM"], club: { id: 7, name: "HQ", market: { timezone: "America/Los_Angeles" } }, address: { formatted: "123 Main St", notes: "Use the side door" }, - userReceipt: { id: 201, due: 4.5, clubCopay: 15 }, + userReceipt: { due: 4.5, clubCopay: 15 }, orders: [ { id: 101, - venue: { id: 301, displayName: "Test Kitchen" }, + venue: { displayName: "Test Kitchen" }, etaStatus: { status: "delayed", start: "2026-08-28T11:45:00-07:00", @@ -55,7 +52,6 @@ const delivery: Delivery = { price: 12.5, group: "A1", isConfirmed: true, - autoOrder: true, isRemoval: true, requestStatus: "pending", nonHiddenAttributes: [{ label: "Protein", value: "Tofu" }], @@ -64,7 +60,7 @@ const delivery: Delivery = { }, { id: 102, - venue: { id: 302, displayName: "Second Kitchen" }, + venue: { displayName: "Second Kitchen" }, etaStatus: { status: "ontime", start: "2026-08-28T12:05:00-07:00", @@ -93,7 +89,6 @@ const menu: Menu = { name: "Test Kitchen", sections: [ { - id: 1, items: [ { id: ITEM_ID, @@ -214,6 +209,7 @@ describe("read tool responses", () => { group: "A1", isConfirmed: true, cancellationPending: true, + rating: null, }); }); @@ -283,25 +279,6 @@ describe("read tool responses", () => { ]); }); - test("pick explanations report ranks rather than raw scores", async () => { - const result = await handlers.get("explain_pick")!({ - deliveryId: DELIVERY_ID, - }); - expect(structured(result)).toEqual({ - picked: [ - { itemId: ITEM_ID, menuId: MENU_ID, name: "Test Bowl", rank: 1 }, - { - itemId: ITEM_ID + 1, - menuId: MENU_ID + 1, - name: "Second Bowl", - rank: null, - }, - ], - top: [{ menuId: MENU_ID, itemId: ITEM_ID, rank: 1, name: "Test Bowl" }], - }); - expect(result.content[0]?.type === "text" ? result.content[0].text : "").not.toContain("score"); - }); - test("delivery status keeps every tracker without returning the full server snapshot", async () => { const result = await handlers.get("get_delivery_status")!({ deliveryId: DELIVERY_ID, @@ -324,6 +301,7 @@ describe("read tool responses", () => { group: "A1", isConfirmed: true, cancellationPending: true, + rating: null, }, { pieceId: "piece-2", @@ -335,6 +313,7 @@ describe("read tool responses", () => { group: "B2", isConfirmed: true, cancellationPending: false, + rating: null, }, ], orders: [ diff --git a/tests/tools-write.test.ts b/tests/tools-write.test.ts index bf438b5..64964f4 100644 --- a/tests/tools-write.test.ts +++ b/tests/tools-write.test.ts @@ -21,7 +21,6 @@ function menu(price: number | null = 12.5): Menu { name: "Test Kitchen", sections: [ { - id: 1, items: [ { id: ITEM_ID, @@ -43,7 +42,7 @@ function delivery( return { id, availableMenuIds: [MENU_ID], - orders: [{ id: id * 100, menu: { id: MENU_ID }, pieces }], + orders: [{ id: id * 100, pieces }], }; } @@ -132,7 +131,6 @@ describe("write tool planning", () => { id: MENU_ID + 1, sections: [ { - id: 2, items: [{ id: ITEM_ID, menuId: MENU_ID + 1, name: "Other Bowl", modifiers: [] }], }, ], @@ -270,12 +268,10 @@ describe("write tool planning", () => { orders: [ { id: 100, - menu: { id: MENU_ID }, pieces: [{ id: "first", itemId: 1, menuId: MENU_ID, userId: USER_ID }], }, { id: 101, - menu: { id: MENU_ID }, pieces: [{ id: "second", itemId: 2, menuId: MENU_ID, userId: USER_ID }], }, ], @@ -437,43 +433,11 @@ describe("write tool planning", () => { expect(dietChecks).toBe(2); }); - test("remove and skip require positive ownership", async () => { + test("remove requires positive ownership", async () => { deliveries = [delivery(1, [{ id: "theirs", itemId: 1, menuId: MENU_ID, userId: USER_ID + 1 }])]; - const results = await Promise.all([ - handlers.get("remove_meal")!({ deliveryId: 1, pieceId: "theirs" }), - handlers.get("skip_delivery")!({ deliveryId: 1 }), - ]); - for (const result of results) { - expect(result.isError).toBe(true); - expect(structured(result).confirmToken).toBeUndefined(); - } - }); - - test("skip explains how to remove multiple positively owned meals", async () => { - deliveries = [ - delivery(1, [ - { id: "first", itemId: 1, menuId: MENU_ID, userId: USER_ID }, - { id: "second", itemId: 2, menuId: MENU_ID, userId: USER_ID }, - ]), - ]; - const ambiguous = await handlers.get("skip_delivery")!({ deliveryId: 1 }); - const message = ambiguous.content[0]?.type === "text" ? ambiguous.content[0].text : ""; - expect(ambiguous.isError).toBe(true); - expect(structured(ambiguous).confirmToken).toBeUndefined(); - expect(message).toContain("remove_meal"); - expect(message).toContain("pieceId"); - expect(message).not.toContain("sourcePieceId"); - expect(mutations).toEqual([]); - - deliveries = [ - delivery(1, [ - { id: "mine", itemId: 1, menuId: MENU_ID, userId: USER_ID }, - { id: "theirs", itemId: 2, menuId: MENU_ID, userId: USER_ID + 1 }, - { id: "unknown", itemId: 3, menuId: MENU_ID }, - ]), - ]; - const oneOwned = await handlers.get("skip_delivery")!({ deliveryId: 1 }); - expect(structured(oneOwned).mode).toBe("preview"); + const result = await handlers.get("remove_meal")!({ deliveryId: 1, pieceId: "theirs" }); + expect(result.isError).toBe(true); + expect(structured(result).confirmToken).toBeUndefined(); }); test("blocks unknown prices only when a preview ceiling is configured", async () => { diff --git a/tests/write-gate.test.ts b/tests/write-gate.test.ts index 669d631..688620c 100644 --- a/tests/write-gate.test.ts +++ b/tests/write-gate.test.ts @@ -1,3 +1,4 @@ +import type { CallToolResult } from "@modelcontextprotocol/server"; import { describe, expect, test } from "bun:test"; import { MutationError, MutationOutcomeUnknownError } from "@/net/errors.ts"; import { @@ -5,7 +6,6 @@ import { hashWriteArgs, type GateCtx, type ExecutableWritePlan, - type ToolResultLike, type WritePlan, } from "@/write-gate.ts"; @@ -42,8 +42,16 @@ function argsHash(overrides: Record = {}): string { return hashWriteArgs({ deliveryId: 1, itemId: 4, ...overrides }); } -function confirmToken(result: ToolResultLike): string { - const token = result.structuredContent?.confirmToken; +function structured(result: CallToolResult): Record { + return (result.structuredContent ?? {}) as Record; +} + +function textOf(result: CallToolResult): string { + return result.content.flatMap((block) => (block.type === "text" ? [block.text] : [])).join("\n"); +} + +function confirmToken(result: CallToolResult): string { + const token = structured(result).confirmToken; if (typeof token !== "string") throw new Error("result has no confirmToken"); return token; } @@ -122,7 +130,7 @@ describe("createWriteGate", () => { }), { tool: "set_meal", argsHash: argsHash(), plan: async () => planned }, ); - expect(preview.structuredContent?.mode).toBe("preview"); + expect(structured(preview).mode).toBe("preview"); expect(confirmToken(preview)).toBe("token-1"); expect(preview.structuredContent).toEqual({ mode: "preview", @@ -132,8 +140,8 @@ describe("createWriteGate", () => { confirmToken: "token-1", expiresAt: "1970-01-01T00:10:01.000Z", }); - expect(preview.content[0]?.text).not.toContain("mutation"); - expect(preview.content[0]?.text).not.toContain("selectionsHash"); + expect(textOf(preview)).not.toContain("mutation"); + expect(textOf(preview)).not.toContain("selectionsHash"); expect(previewExecutions).toBe(0); planned.input.itemId = 999; @@ -191,8 +199,8 @@ describe("createWriteGate", () => { }); expect(planned).toBe(1); expect(replay.isError).toBe(true); - expect(replay.structuredContent?.mode).toBe("preview"); - expect(replay.structuredContent?.confirmationError).toEqual({ + expect(structured(replay).mode).toBe("preview"); + expect(structured(replay).confirmationError).toEqual({ reason: "unknown", message: "confirmToken is unknown or has already been used. Here is a fresh preview:", }); @@ -215,7 +223,7 @@ describe("createWriteGate", () => { plan: async () => clonePlan(), }); expect(mismatch.isError).toBe(true); - expect(mismatch.content[0]?.text).toContain("does not match these tool arguments"); + expect(textOf(mismatch)).toContain("does not match these tool arguments"); const afterMismatch = await gate(context(), { tool: "set_meal", @@ -223,7 +231,7 @@ describe("createWriteGate", () => { confirmToken: token, plan: async () => clonePlan(), }); - expect(afterMismatch.content[0]?.text).toContain("unknown or has already been used"); + expect(textOf(afterMismatch)).toContain("unknown or has already been used"); }); test("actor, delegation, and tool are part of the pending-write binding", async () => { @@ -239,7 +247,7 @@ describe("createWriteGate", () => { confirmToken: confirmToken(actorPreview), plan: async () => clonePlan(), }); - expect(wrongActor.content[0]?.text).toContain("different Forkable user or delegation"); + expect(textOf(wrongActor)).toContain("different Forkable user or delegation"); const delegationPreview = await gate(context(42, undefined, "delegation-a"), { tool: "set_meal", @@ -252,7 +260,7 @@ describe("createWriteGate", () => { confirmToken: confirmToken(delegationPreview), plan: async () => clonePlan(), }); - expect(wrongDelegation.content[0]?.text).toContain("different Forkable user or delegation"); + expect(textOf(wrongDelegation)).toContain("different Forkable user or delegation"); const toolPreview = await gate(context(42), { tool: "set_meal", @@ -265,7 +273,7 @@ describe("createWriteGate", () => { confirmToken: confirmToken(toolPreview), plan: async () => clonePlan(), }); - expect(wrongTool.content[0]?.text).toContain("different tool"); + expect(textOf(wrongTool)).toContain("different tool"); }); test("evicts the oldest pending write at the cap", async () => { @@ -291,7 +299,7 @@ describe("createWriteGate", () => { confirmToken: confirmToken(first), plan: async () => clonePlan(), }); - expect(evicted.content[0]?.text).toContain("unknown"); + expect(textOf(evicted)).toContain("unknown"); expect(confirmToken(second)).toBe("token-2"); }); @@ -310,7 +318,7 @@ describe("createWriteGate", () => { confirmToken: confirmToken(preview), plan: async () => clonePlan(), }); - expect(result.content[0]?.text).toContain("expired"); + expect(textOf(result)).toContain("expired"); }); test("blocking guards never issue a token or resolve an actor", async () => { @@ -329,15 +337,15 @@ describe("createWriteGate", () => { guards: [{ code: "selection_invalid", level: "block", message: "No write" }], }), }); - expect(result.structuredContent?.mode).toBe("blocked"); + expect(structured(result).mode).toBe("blocked"); expect(result.structuredContent).not.toHaveProperty("confirmToken"); expect(result.structuredContent).not.toHaveProperty("variables"); - expect(result.content[0]?.text).not.toContain("Would-be mutation"); + expect(textOf(result)).not.toContain("Would-be mutation"); expect(actorResolutions).toBe(0); }); test("maps definite and uncertain mutation failures", async () => { - const resultFor = async (error: Error): Promise => { + const resultFor = async (error: Error): Promise => { const gate = deterministicGate(); const preview = await gate(context(), { tool: "set_meal", @@ -365,20 +373,20 @@ describe("createWriteGate", () => { }), ); expect(rejected.isError).toBe(true); - expect(rejected.structuredContent?.mode).toBe("rejected"); - expect(rejected.structuredContent?.reasons).toEqual(["not allowed", "venue_capacity_overage"]); + expect(structured(rejected).mode).toBe("rejected"); + expect(structured(rejected).reasons).toEqual(["not allowed", "venue_capacity_overage"]); expect(rejected.structuredContent).not.toHaveProperty("errorDetails"); const unknown = await resultFor( new MutationOutcomeUnknownError("replacePiece", "connection closed", 502), ); expect(unknown.isError).toBe(true); - expect(unknown.structuredContent?.mode).toBe("outcome_unknown"); + expect(structured(unknown).mode).toBe("outcome_unknown"); expect(unknown.structuredContent).not.toHaveProperty("status"); - expect(unknown.structuredContent?.retrySafe).toBe(false); - expect(unknown.structuredContent?.message).toBe("connection closed"); - expect(unknown.content[0]?.text).toStartWith("Outcome unknown: connection closed."); - expect(unknown.structuredContent?.reconciliation).toEqual({ + expect(structured(unknown).retrySafe).toBe(false); + expect(structured(unknown).message).toBe("connection closed"); + expect(textOf(unknown)).toStartWith("Outcome unknown: connection closed."); + expect(structured(unknown).reconciliation).toEqual({ tool: "list_deliveries", deliveryIds: [1], }); From d580ebcc955db3656e1b11a0da05e0b2c4629f74 Mon Sep 17 00:00:00 2001 From: colinds <90475914+colinds@users.noreply.github.com> Date: Tue, 8 Sep 2026 13:47:15 -0700 Subject: [PATCH 2/3] Address meal rating review findings --- CLAUDE.md | 19 +++-- README.md | 7 +- skills/forkable/SKILL.md | 10 ++- src/tools.ts | 30 ++++++-- tests/tools-rating.test.ts | 150 +++++++++++++++++++++++++++++++------ tests/tools-write.test.ts | 20 +++++ 6 files changed, 193 insertions(+), 43 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 0a3a57a..0f632c0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -146,19 +146,27 @@ fields has returned HTTP 503. Keep `confirmDelivery` on its known selection. `rate_meal` uses the authenticated dashboard's `rateMeal` mutation with selection `errors`. Resolve `deliveryId` and a unique, positively owned `pieceId`, then send `piece.userRating.id` as `id` with `channel: "mc"`. A missing rating record or id is unavailable; never invent one. Buffet -ratings use a different flow and are unsupported here. +ratings use a different flow and are unsupported here. The captured dashboard's `MealRating.save` +sends `attachment` as null or the stored URL and requests only `errors` from `rateMeal`. Preserve +that known request shape; the `errorDetails` behavior observed on meal-order mutations does not +establish support for that field on rating mutations. The dashboard's textarea sends a string for +comments, including empty strings. Server persistence of clearing edits has not been live-tested. Scores are integers from 1–5. Levels 4–5 use the dashboard's compliment codes; 1–3 use its issue -codes. Explicit incompatible reasons are rejected. A category change filters incompatible stored -reasons. Omitted feedback preserves existing values; explicit empty reasons or comments clear them. +codes. Explicit incompatible reasons are rejected. Omitted reasons retain unknown server codes and +drop only known incompatible codes, even when the stored level is absent. Duplicate reasons are +removed. Other omitted feedback preserves existing values; explicit empty reasons or comments +clear them. Preserve existing attachments, and do not call `updateUser` or invent follow-up consent. Read projections expose nullable `rating` objects with level, reasons, comment, guest flag, and follow-up preference. No record means unavailable; a record without a level means unrated. Mutation IDs, channel, and attachments stay internal. Ratings and meals always use the same ownership filter. -Rating previews search from 14 days ago by default and accept `from` for older meals. The stored -plan includes `reconciliationRange`; an uncertain outcome returns it as `reconciliation.arguments` +Rating previews search from 14 days ago by default and accept `from` and `to` for bounded older +lookups, rejecting backwards ranges before making requests. Score-change previews show the old +and new score. The stored plan includes `reconciliationRange`; an uncertain outcome returns it as +`reconciliation.arguments` for `list_deliveries`. Preserve this range through the gate's structured clone and confirmation. ## `selectionsHash` @@ -261,7 +269,6 @@ Other wire constraints: - `delivery.state`, `delivery.simpleState`, and `order.state` are different lifecycles; do not merge them into one source field. - `me.roles` is a JSON feature-flags scalar, not a role-name array. -- `Piece.autoOrder` reflects account auto-order behavior, not who selected the meal. - `club.hidePrices` is a Forkable display preference, not an API authorization boundary. ## Environment diff --git a/README.md b/README.md index 289bd44..e93e757 100644 --- a/README.md +++ b/README.md @@ -105,12 +105,13 @@ List the delivery first, supplying `from` and `to` for past meals. Each owned me Use `rate_meal` with the delivery ID, the meal's `pieceId`, and a `level` from 1–5. It previews first; call again with the same arguments and its `confirmToken` to submit. The search starts 14 days ago -by default; pass `from` to rate an older meal. +by default; pass both `from` and `to` to limit the lookup to an older day or week. Optional `reasons`, `comment`, `forGuest`, and `allowRatingFollowUps` edit the feedback. Omitted fields keep their current values; an empty comment or reason array clears it. Levels 4–5 accept compliment -codes, while 1–3 accept issue codes listed in the tool schema. Changing score categories removes -incompatible stored reasons. Marking a meal as a guest meal excludes its rating from your future +codes, while 1–3 accept issue codes listed in the tool schema. Known incompatible stored reasons are +removed, unknown server codes are preserved, and duplicates are ignored. Score changes show both +the old and new score in the preview. Marking a meal as a guest meal excludes its rating from future suggestions. Follow-up preferences apply to this rating without changing your account settings. Existing attachments are kept; photo editing and buffet ratings are not supported. diff --git a/skills/forkable/SKILL.md b/skills/forkable/SKILL.md index add6420..bb22180 100644 --- a/skills/forkable/SKILL.md +++ b/skills/forkable/SKILL.md @@ -89,13 +89,15 @@ mutation. Use `rate_meal` for a 1–5 score or feedback edits on one owned meal. List past deliveries with explicit `from` and `to`, then use the returned `deliveryId` and `pieceId`. The rating tool searches from 14 -days ago by default; pass `from` for older meals. A meal's `rating: null` means unavailable, while a -rating object with `level: null` means unrated. Do not infer rating availability from delivery status. +days ago by default; pass both `from` and `to` to bound older lookups to the meal's day or week. +A meal's `rating: null` means unavailable, while a rating object with `level: null` means unrated. +Do not infer rating availability from delivery status. Ask for the user's actual score and feedback; do not manufacture a rating from their food preferences. Optional reasons use the tool schema's compliment codes for 4–5 and issue codes for 1–3. Omitted -feedback stays unchanged; explicit empty comments or reason arrays clear it. When switching score -categories, incompatible stored reasons are removed. Existing attachments are kept. +feedback stays unchanged; explicit empty comments or reason arrays clear it. Known incompatible +stored reasons are removed, unknown server codes are preserved, and duplicates are ignored. +Existing attachments are kept. Check the old and new score shown in a score-change preview. `forGuest: true` excludes this rating from the user's future meal suggestions. Set `allowRatingFollowUps` only when the user states a preference; it applies to this rating and does not diff --git a/src/tools.ts b/src/tools.ts index fbc3e9e..561b00a 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -1290,6 +1290,9 @@ export function registerAllTools(server: McpServer, writeGate: WriteGate): void .optional() .describe("Allow Forkable to follow up on this rating; does not change account settings"), from: dateArg().optional().describe("Search start (YYYY-MM-DD); defaults to 14 days ago"), + to: dateArg() + .optional() + .describe("Inclusive search end (YYYY-MM-DD); pass with from for older meals"), confirmToken: z.string().optional(), }), annotations: { destructiveHint: true, idempotentHint: false, openWorldHint: true }, @@ -1297,7 +1300,10 @@ export function registerAllTools(server: McpServer, writeGate: WriteGate): void async (a) => guard(async (client, session) => { const plan = async (): Promise => { - const range = deliveryRange(a.from ?? dateOffsetLocal(-14)); + const range = deliveryRange(a.from ?? dateOffsetLocal(-14), a.to); + if (range.to < range.from) { + throw new Error(`Window ends before it starts: ${range.from} → ${range.to}.`); + } const { deliveries, userId } = await loadDeliveries( client, range.from, @@ -1321,18 +1327,26 @@ export function registerAllTools(server: McpServer, writeGate: WriteGate): void ); } const allowed: readonly string[] = a.level >= 4 ? RATING_COMPLIMENTS : RATING_ISSUES; + const opposite: readonly string[] = a.level >= 4 ? RATING_ISSUES : RATING_COMPLIMENTS; if (a.reasons?.some((reason) => !allowed.includes(reason))) { throw new Error(`A ${a.level}/5 rating accepts these reasons: ${allowed.join(", ")}.`); } - const changedCategory = rating.level != null && rating.level >= 4 !== a.level >= 4; - const reasons = - a.reasons ?? - (changedCategory - ? (rating.reasons ?? []).filter((reason) => allowed.includes(reason)) - : (rating.reasons ?? [])); + // Preserve server codes we do not recognize, removing only known incompatible reasons. + const reasons = [ + ...new Set( + a.reasons ?? + (rating.reasons ?? []).filter( + (reason) => allowed.includes(reason) || !opposite.includes(reason), + ), + ), + ]; const comment = a.comment ?? rating.comment ?? null; const forGuest = a.forGuest ?? rating.forGuest; const followUps = a.allowRatingFollowUps ?? rating.allowRatingFollowUps; + const score = + rating.level != null && rating.level !== a.level + ? `change score from ${rating.level}/5 to ${a.level}/5` + : `${a.level}/5`; return { op: "rateMeal", selection: "errors", @@ -1348,7 +1362,7 @@ export function registerAllTools(server: McpServer, writeGate: WriteGate): void }, summary: `Rate ${piece.name ?? `meal ${piece.id}`} (piece ${piece.id}) on delivery ${d.id} ` + - `(${formatDay(d.forDeliveryAt)}) ${a.level}/5; reasons: ${reasons.join(", ") || "none"}; ` + + `(${formatDay(d.forDeliveryAt)}) ${score}; reasons: ${reasons.join(", ") || "none"}; ` + `comment: ${comment ? JSON.stringify(comment) : "none"}; guest meal: ${ratingFlag(forGuest)}; ` + `allow follow-ups: ${ratingFlag(followUps)}${rating.attachment ? "; existing attachment kept" : ""}`, deliveryIds: [d.id], diff --git a/tests/tools-rating.test.ts b/tests/tools-rating.test.ts index 33bb4da..b9379c3 100644 --- a/tests/tools-rating.test.ts +++ b/tests/tools-rating.test.ts @@ -11,7 +11,13 @@ import { createWriteGate } from "@/write-gate.ts"; type Handler = (args: Record) => Promise; const USER_ID = 42; -const ratingArgs = { deliveryId: 1, pieceId: "mine", level: 5, from: "2026-08-01" }; +const ratingArgs = { + deliveryId: 1, + pieceId: "mine", + level: 5, + from: "2026-08-01", + to: "2026-08-31", +}; const structured = (result: CallToolResult) => (result.structuredContent ?? {}) as Record; const textOf = (result: CallToolResult) => @@ -30,6 +36,13 @@ describe("meal ratings", () => { let delivery: Delivery; let handlers: Map; let queries: string[]; + let requests: { + url: string; + method?: string; + redirect?: RequestRedirect; + headers: Headers; + mutation: boolean; + }[]; let mutations: { query: string; variables: { input: Record } }[]; let mutationResponse: Record | Error; let reportedUserId: number | null; @@ -45,7 +58,15 @@ describe("meal ratings", () => { menuId: 10, userId: USER_ID, name: "Lunch bowl", - userRating: { id: 500, level: null, reasons: [], comment: null }, + userRating: { + id: 500, + level: null, + reasons: [], + comment: null, + attachment: null, + forGuest: null, + allowRatingFollowUps: null, + }, }; delivery = { id: 1, @@ -53,19 +74,22 @@ describe("meal ratings", () => { orders: [{ id: 100, pieces: [piece] }], }; queries = []; + requests = []; mutations = []; mutationResponse = { data: { rateMeal: { errors: [] } } }; reportedUserId = USER_ID; originalFetch = globalThis.fetch; globalThis.fetch = (async (url: string, init?: RequestInit) => { - expect(url).toBe(ENDPOINT); const body = JSON.parse(init?.body as string); + requests.push({ + url, + method: init?.method, + redirect: init?.redirect, + headers: new Headers(init?.headers), + mutation: body.query.startsWith("mutation"), + }); if (body.query.startsWith("mutation")) { mutations.push(body); - expect(init?.redirect).toBe("manual"); - const headers = new Headers(init?.headers); - expect(headers.get("cookie")).toBe("_easyorder_session=test"); - expect(headers.get("x-csrf-token")).toBe("test-csrf"); if (mutationResponse instanceof Error) throw mutationResponse; return Response.json(mutationResponse); } @@ -100,6 +124,14 @@ describe("meal ratings", () => { if (originalHome === undefined) delete process.env.FORKABLE_MCP_HOME; else process.env.FORKABLE_MCP_HOME = originalHome; rmSync(home, { recursive: true, force: true }); + // Assert outside fetch so the client cannot swallow assertion failures as transport errors. + for (const request of requests) { + expect(request.url).toBe(ENDPOINT); + expect(request.method).toBe("POST"); + expect(request.redirect).toBe(request.mutation ? "manual" : "follow"); + expect(request.headers.get("cookie")).toBe("_easyorder_session=test"); + expect(request.headers.get("x-csrf-token")).toBe("test-csrf"); + } }); const callRating = (overrides: Record = {}) => @@ -121,7 +153,9 @@ describe("meal ratings", () => { expect(mutations).toEqual([ { query: "mutation ($input: RateMealInput!) { rateMeal(input: $input) { errors } }", - variables: { input: { id: 500, level: 5, reasons: [], comment: null, channel: "mc" } }, + variables: { + input: { id: 500, level: 5, reasons: [], comment: null, attachment: null, channel: "mc" }, + }, }, ]); expect(queries.filter((query) => query.includes("myDeliveries"))).toHaveLength(1); @@ -142,6 +176,7 @@ describe("meal ratings", () => { }; const preview = await callRating(); expect(textOf(preview)).toContain('comment: "Keep this"'); + expect(textOf(preview)).toContain("change score from 4/5 to 5/5"); expect(textOf(preview)).toContain( "guest meal: yes; allow follow-ups: no; existing attachment kept", ); @@ -158,11 +193,18 @@ describe("meal ratings", () => { comment: "Old comment", forGuest: true, allowRatingFollowUps: true, + attachment: null, }; const edit = { reasons: [], comment: "", forGuest: false, allowRatingFollowUps: false }; const preview = await callRating(edit); await callRating({ ...edit, confirmToken: tokenOf(preview) }); - expect(mutations[0]!.variables.input).toEqual({ id: 500, level: 5, channel: "mc", ...edit }); + expect(mutations[0]!.variables.input).toEqual({ + id: 500, + level: 5, + channel: "mc", + attachment: null, + ...edit, + }); }); test.each([ @@ -184,12 +226,51 @@ describe("meal ratings", () => { }, ); + test.each([ + { + previous: 5, + next: 2, + reasons: ["excellent_food", "new_server_reason", "other"], + kept: ["new_server_reason", "other"], + }, + { + previous: null, + next: 4, + reasons: ["food_quality", "new_server_reason", "other"], + kept: ["new_server_reason", "other"], + }, + { + previous: 2, + next: 2, + reasons: ["excellent_food", "food_temp", "new_server_reason"], + kept: ["food_temp", "new_server_reason"], + }, + ])( + "preserves unknown reasons and removes known incompatible ones from $previous to $next", + async ({ previous, next, reasons, kept }) => { + piece.userRating = { ...piece.userRating!, level: previous, reasons: [...reasons] }; + const preview = await callRating({ level: next }); + await callRating({ level: next, confirmToken: tokenOf(preview) }); + expect(mutations[0]!.variables.input.reasons).toEqual(kept); + }, + ); + + test.each([false, true])("deduplicates reasons (explicit=%s)", async (explicit) => { + const reasons = ["other", "excellent_food", "other"]; + piece.userRating!.reasons = reasons; + const edit = explicit ? { reasons } : {}; + const preview = await callRating(edit); + await callRating({ ...edit, confirmToken: tokenOf(preview) }); + expect(mutations[0]!.variables.input.reasons).toEqual(["other", "excellent_food"]); + }); + test.each([ { level: 5, reasons: ["food_quality"] }, { level: 2, reasons: ["excellent_food"] }, ])("refuses incompatible explicit reasons for $level without a token", async (edit) => { const result = await callRating(edit); expect(result.isError).toBe(true); + expect(textOf(result)).toStartWith(`Error: A ${edit.level}/5 rating accepts these reasons:`); expect(structured(result).confirmToken).toBeUndefined(); expect(mutations).toEqual([]); }); @@ -200,6 +281,7 @@ describe("meal ratings", () => { { level: 2.5 }, { reasons: ["invented_reason"] }, { from: "2026-02-30" }, + { to: "2026-02-30" }, ])("rejects invalid input %j at the tool schema", async (edit) => { await expect(callRating(edit)).rejects.toThrow(); expect(queries).toEqual([]); @@ -207,16 +289,16 @@ describe("meal ratings", () => { }); test.each([ - "missing", - "duplicate", - "other_owner", - "unknown_owner", - "unavailable", - "empty_id", - "missing_actor", - "buffet", - "wrong_delivery", - ])("does not preview an unsafe or unavailable target: %s", async (scenario) => { + ["missing", "Piece mine was not found uniquely on delivery 1."], + ["duplicate", "Piece mine was not found uniquely on delivery 1."], + ["other_owner", "Piece mine is not verified as belonging to you."], + ["unknown_owner", "Piece mine is not verified as belonging to you."], + ["unavailable", "Forkable has not made a rating available for meal mine on delivery 1."], + ["empty_id", "Forkable has not made a rating available for meal mine on delivery 1."], + ["missing_actor", "Forkable did not report your user id."], + ["buffet", "Buffet ratings are not supported; use Forkable to rate this delivery."], + ["wrong_delivery", "Delivery 1 not found between 2026-08-01 and 2026-08-31."], + ])("does not preview an unsafe or unavailable target: %s", async (scenario, message) => { if (scenario === "missing") delivery.orders![0]!.pieces = []; if (scenario === "duplicate") delivery.orders![0]!.pieces!.push({ ...piece }); if (scenario === "other_owner") piece.userId = 99; @@ -228,6 +310,7 @@ describe("meal ratings", () => { if (scenario === "wrong_delivery") delivery.id = 2; const result = await callRating(); expect(result.isError).toBe(true); + expect(textOf(result)).toBe(`Error: ${message}`); expect(structured(result).confirmToken).toBeUndefined(); expect(mutations).toEqual([]); }); @@ -258,11 +341,28 @@ describe("meal ratings", () => { }); test("searches recent history by default and accepts an older explicit window", async () => { - await callRating({ from: undefined }); + await callRating({ from: undefined, to: undefined }); const range = deliveryRange(addDaysLocal(new Date().toLocaleDateString("en-CA"), -14)); expect(queries[0]).toContain(`from: "${range.from}", to: "${range.to}"`); await callRating(); - expect(queries.find((query) => query.includes('from: "2026-08-01"'))).toBeDefined(); + expect( + queries.find((query) => query.includes('from: "2026-08-01", to: "2026-08-31"')), + ).toBeDefined(); + }); + + test("rejects backwards rating windows before any request", async () => { + const result = await callRating({ from: "2026-08-31", to: "2026-08-01" }); + expect(result.isError).toBe(true); + expect(textOf(result)).toBe("Error: Window ends before it starts: 2026-08-31 → 2026-08-01."); + expect(structured(result).confirmToken).toBeUndefined(); + expect(requests).toEqual([]); + }); + + test("binds the historical end date to confirmation", async () => { + const preview = await callRating(); + const result = await callRating({ to: "2026-08-30", confirmToken: tokenOf(preview) }); + expect(structured(result).confirmationError.reason).toBe("args_changed"); + expect(mutations).toEqual([]); }); test("returns historical reconciliation without replaying an uncertain mutation", async () => { @@ -275,7 +375,7 @@ describe("meal ratings", () => { reconciliation: { tool: "list_deliveries", deliveryIds: [1], - arguments: deliveryRange(ratingArgs.from), + arguments: { from: ratingArgs.from, to: ratingArgs.to }, }, }); expect(mutations).toHaveLength(1); @@ -293,6 +393,12 @@ describe("meal ratings", () => { expect(mutations).toHaveLength(1); }); + test.each([null, 2])("renders the meal rating level %j", async (level) => { + piece.userRating!.level = level; + const result = await handlers.get("get_delivery_status")!({ deliveryId: 1 }); + expect(textOf(result)).toContain(level == null ? "Rating : not rated" : "Rating : 2/5"); + }); + test("list and status expose only owned feedback, distinguishing unavailable from unrated", async () => { delivery.orders![0]!.pieces!.push({ ...piece, id: "not-available", userRating: null }); delivery.orders![0]!.pieces!.push({ diff --git a/tests/tools-write.test.ts b/tests/tools-write.test.ts index 64964f4..449f9a8 100644 --- a/tests/tools-write.test.ts +++ b/tests/tools-write.test.ts @@ -440,6 +440,26 @@ describe("write tool planning", () => { expect(structured(result).confirmToken).toBeUndefined(); }); + test("remove refuses duplicate piece IDs across orders", async () => { + const piece = { id: 7, itemId: 1, menuId: MENU_ID, userId: USER_ID }; + deliveries = [ + { + id: 1, + orders: [ + { id: 100, pieces: [piece] }, + { id: 101, pieces: [{ ...piece, id: "7" }] }, + ], + }, + ]; + const result = await handlers.get("remove_meal")!({ deliveryId: 1, pieceId: "7" }); + expect(result.isError).toBe(true); + expect(result.content[0]?.type === "text" ? result.content[0].text : "").toBe( + "Error: Piece 7 was not found uniquely on delivery 1.", + ); + expect(structured(result).confirmToken).toBeUndefined(); + expect(mutations).toEqual([]); + }); + test("blocks unknown prices only when a preview ceiling is configured", async () => { menus = [menu(null)]; const withoutCeiling = await handlers.get("set_meal")!({ From 1f4e9f7f378b632fab875f0035bde562a3d9a523 Mon Sep 17 00:00:00 2001 From: colinds <90475914+colinds@users.noreply.github.com> Date: Tue, 8 Sep 2026 19:45:17 -0700 Subject: [PATCH 3/3] Clarify Forkable rating follow-up defaults after live validation --- CLAUDE.md | 5 +++++ README.md | 3 +++ skills/forkable/SKILL.md | 4 +++- src/tools.ts | 6 ++++-- tests/tools-rating.test.ts | 3 ++- 5 files changed, 17 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 0f632c0..97fd13f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -152,6 +152,11 @@ that known request shape; the `errorDetails` behavior observed on meal-order mut establish support for that field on rating mutations. The dashboard's textarea sends a string for comments, including empty strings. Server persistence of clearing edits has not been live-tested. +A live initial rating submission and readback succeeded with the null attachment and `errors`-only +payload selection. An omitted `allowRatingFollowUps` changed from unreported to `true` in the +readback. Describe an unreported preference as using Forkable's default, not as remaining unchanged; +do not invent a local default or change the account-wide preference. + Scores are integers from 1–5. Levels 4–5 use the dashboard's compliment codes; 1–3 use its issue codes. Explicit incompatible reasons are rejected. Omitted reasons retain unknown server codes and drop only known incompatible codes, even when the stored level is absent. Duplicate reasons are diff --git a/README.md b/README.md index e93e757..375884f 100644 --- a/README.md +++ b/README.md @@ -113,6 +113,9 @@ codes, while 1–3 accept issue codes listed in the tool schema. Known incompati removed, unknown server codes are preserved, and duplicates are ignored. Score changes show both the old and new score in the preview. Marking a meal as a guest meal excludes its rating from future suggestions. Follow-up preferences apply to this rating without changing your account settings. +If Forkable has not reported a follow-up preference, omitting it lets Forkable apply its default, +which may allow its team to contact you about your feedback. Set `allowRatingFollowUps: false` +to opt out for the rating. Existing attachments are kept; photo editing and buffet ratings are not supported. ## Authentication diff --git a/skills/forkable/SKILL.md b/skills/forkable/SKILL.md index bb22180..7c64222 100644 --- a/skills/forkable/SKILL.md +++ b/skills/forkable/SKILL.md @@ -102,7 +102,9 @@ Existing attachments are kept. Check the old and new score shown in a score-chan `forGuest: true` excludes this rating from the user's future meal suggestions. Set `allowRatingFollowUps` only when the user states a preference; it applies to this rating and does not change account settings. Show the exact score, feedback, and preferences in the preview before -confirming. Use delivery lists and recommendations to compare meals; no tool explains the model's +confirming. If the preference is unreported, explain that omission lets Forkable apply its default, +which may enable contact about the rating. Do not describe that as preserving a known preference. +Use delivery lists and recommendations to compare meals; no tool explains the model's reasoning or reports ranks beyond the returned recommendations. ## Dietary advisory diff --git a/src/tools.ts b/src/tools.ts index 561b00a..be094ba 100644 --- a/src/tools.ts +++ b/src/tools.ts @@ -129,7 +129,7 @@ const RATING_ISSUES = [ ] as const; function ratingFlag(value: boolean | null | undefined): string { - if (value == null) return "unchanged (not reported)"; + if (value == null) return "Forkable default (not reported)"; return value ? "yes" : "no"; } @@ -1288,7 +1288,9 @@ export function registerAllTools(server: McpServer, writeGate: WriteGate): void allowRatingFollowUps: z .boolean() .optional() - .describe("Allow Forkable to follow up on this rating; does not change account settings"), + .describe( + "Allow Forkable to contact you about this rating; omission keeps a reported preference or uses Forkable's default, which may enable follow-ups", + ), from: dateArg().optional().describe("Search start (YYYY-MM-DD); defaults to 14 days ago"), to: dateArg() .optional() diff --git a/tests/tools-rating.test.ts b/tests/tools-rating.test.ts index b9379c3..36effd7 100644 --- a/tests/tools-rating.test.ts +++ b/tests/tools-rating.test.ts @@ -142,7 +142,8 @@ describe("meal ratings", () => { expect(structured(preview).mode).toBe("preview"); expect(textOf(preview)).toContain("Lunch bowl (piece mine)"); expect(textOf(preview)).toContain("5/5"); - expect(textOf(preview)).toContain("allow follow-ups: unchanged (not reported)"); + expect(textOf(preview)).toContain("allow follow-ups: Forkable default (not reported)"); + expect(textOf(preview)).not.toContain("unchanged"); expect(textOf(preview)).not.toContain("500"); expect(mutations).toEqual([]); expect(queries.filter((query) => query.includes("myDeliveries"))).toHaveLength(1);