diff --git a/README.md b/README.md index d294cfb..2eec94a 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,7 @@ internet said. | **See what it holds about you** | `/privacy` lists everything stored about you: facts, remembered conversation, scheduled questions, alerts, notification setting, voice minutes, attachments on your archived messages, suggestions, access tokens, archived message counts in the channels you can read now, and how many of your questions are traced. In a server channel it is private and shows counts and fact kinds only, with a button that sends the values by DM; in a DM it shows the values. It states how long questions and answers are traced (`TRACE_RETENTION_DAYS`), that admins can read them, and what survives a deletion, including backups (`BACKUP_RETENTION_DAYS`) | | **Delete everything about you** | `/privacy` -> **Delete everything…** erases your messages and their attachments, facts, memory, tasks, alerts, notifications, suggestions, tokens and voice usage records, and schedules your traced questions for deletion, after you type `DELETE` (`APAGAR` in Portuguese). Choose to keep using the bot (new messages are archived, nothing older is re-imported) or to also stop being archived. It finishes even if the bot restarts midway, and says what is kept (see `docs/operations.md`) | | **Scheduled questions** | `/schedule` asks something for you hourly to daily and messages you the answer — only when there is one. Off by default | -| **Feature requests** | `/suggest` records an idea in your own words (up to 1000 characters) and answers with its number; `/suggestions` lists yours and their status. A message starting "tenho uma sugestão", "sugestão:", "seria legal se você", "I have a feature request", "feature request:" or "it would be nice if you could" is offered with [Record suggestion] and [No, answer it], unless something else the bot does answers it. The team sees the text and your Discord name. Text with an email, phone number or wallet address is refused, resubmitting is idempotent, and five a day at most. Opt-out deletes them | +| **Feature requests** | `/suggest` records an idea in your own words (up to 1000 characters) and answers with its number; `/suggestions` lists yours and their status. A message starting "tenho uma sugestão", "sugestão:", "seria legal se você", "I have a feature request", "feature request:" or "it would be nice if you could" is offered with [Record suggestion] and [No, answer it], unless something else the bot does answers it. The team sees the text and your Discord name. Text with an email, phone number or wallet address is refused, resubmitting is idempotent, and five a day at most. Admins triage them in the console (every change audited); if you pressed Yes you get one DM per status change, unless you turned `/notifications` off or your DMs are closed. Opt-out deletes them | | **Voice questions** | Send the bot a voice message in a DM and get an answer as if you had typed it, with a small quoted line of what it understood. Transcribed by `gpt-4o-mini-transcribe`; the audio is never stored. Hard monthly caps per person and for the server. Off by default | | **Channel media (recording only)** | From `MEDIA_ENABLED_AT`, voice notes and images posted in indexed channels are recorded as pending rows: metadata and a CDN link, nothing downloaded and nothing searchable yet. Transcribing voice notes and reading images come later. Off by default | | **Admin console** | A web console for federation, channels, retention, opt-outs, tokens, and usage and estimated cost per person, feature, model and tool, read live from Langfuse (a person's question text only for admins signed in through CyberdyneAuth, audited, and only after their disclosure notice) | diff --git a/console/src/console.e2e.test.ts b/console/src/console.e2e.test.ts index 556db27..269fa65 100644 --- a/console/src/console.e2e.test.ts +++ b/console/src/console.e2e.test.ts @@ -362,6 +362,28 @@ describe("the tokens screen", () => { }); }); +describe("the feature requests screen", () => { + beforeEach(() => open("Feature requests", "Tell me when someone mentions me")); + + it("shows who suggested it and how many others did", () => { + expect(within(rowOf("Sam")).getByText("also suggested by 2 other(s)")).toBeTruthy(); + }); + + it("sends only what the admin changed", async () => { + await fireEvent.click(button("Triage")); + await fireEvent.change(screen.getByLabelText("Status"), { target: { value: "planned" } }); + await fireEvent.input(screen.getByLabelText("Note"), { target: { value: "Q4" } }); + await fireEvent.click(button("Save")); + await screen.findByText("#12 saved."); + expect(api.writes).toEqual(['PATCH /api/feature-requests/12 {"status":"planned","admin_note":"Q4"}']); + }); + + it("filters by status, and says when a status has nothing", async () => { + await fireEvent.click(button("done")); + await screen.findByText("No done suggestions."); + }); +}); + describe("the audit screen", () => { // The reason is shown only here; the setting's name is on the status screen too. beforeEach(() => open("Audit", "credentials are read from the environment and cannot be stored")); @@ -451,6 +473,10 @@ describe("signing in with CyberdyneAuth", () => { await fireEvent.click(screen.getByRole("link", { name: "Tokens" })); await screen.findByText("sam's laptop"); expect(screen.queryByRole("button", { name: "Revoke" })).toBeNull(); + + await fireEvent.click(screen.getByRole("link", { name: "Feature requests" })); + await screen.findByText("Tell me when someone mentions me"); + expect(screen.queryByRole("button", { name: "Triage" })).toBeNull(); expect(api.writes).toEqual([]); }); diff --git a/console/src/domain/featureRequests.test.ts b/console/src/domain/featureRequests.test.ts new file mode 100644 index 0000000..48def15 --- /dev/null +++ b/console/src/domain/featureRequests.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from "vitest"; + +import { draftOf, draftProblem, patchFor } from "./featureRequests"; +import type { FeatureRequest } from "./types"; + +const ROW: FeatureRequest = { + id: 12, + text: "dark mode", + language: "en", + status: "new", + admin_note: null, + duplicate_of: null, + source_kind: "command", + person: "Sam", + same_text_elsewhere: 0, + created_at: "2026-09-15T10:00:00+00:00", + updated_at: "2026-09-15T10:00:00+00:00", + updated_by: null, +}; + +describe("patchFor", () => { + it("sends nothing for an untouched draft", () => { + expect(patchFor(ROW, draftOf(ROW))).toEqual({}); + const noted = { ...ROW, admin_note: "Q4", duplicate_of: 3 }; + expect(patchFor(noted, draftOf(noted))).toEqual({}); + }); + + it("sends only what changed", () => { + expect(patchFor(ROW, { ...draftOf(ROW), status: "planned" })).toEqual({ status: "planned" }); + expect(patchFor(ROW, { ...draftOf(ROW), note: " Q4 " })).toEqual({ admin_note: "Q4" }); + expect(patchFor(ROW, { ...draftOf(ROW), duplicateOf: "3" })).toEqual({ duplicate_of: 3 }); + }); + + it("clears a note or a link emptied in the form", () => { + const noted = { ...ROW, admin_note: "Q4", duplicate_of: 3 }; + expect(patchFor(noted, { ...draftOf(noted), note: " ", duplicateOf: "" })).toEqual({ + admin_note: null, + duplicate_of: null, + }); + }); +}); + +describe("draftProblem", () => { + it("refuses a duplicate link that is not a number or is the row itself", () => { + expect(draftProblem({ ...draftOf(ROW), duplicateOf: "abc" })).toMatch(/number/); + expect(draftProblem({ ...draftOf(ROW), duplicateOf: "12" })).toMatch(/itself/); + expect(draftProblem({ ...draftOf(ROW), duplicateOf: "3" })).toBeNull(); + }); +}); diff --git a/console/src/domain/featureRequests.ts b/console/src/domain/featureRequests.ts new file mode 100644 index 0000000..cde1262 --- /dev/null +++ b/console/src/domain/featureRequests.ts @@ -0,0 +1,55 @@ +/** + * Feature requests: the statuses, and what an admin's edit sends. + * + * Only the fields that changed are sent, so the change record holds one + * entry per thing the admin actually changed and an untouched note is never + * rewritten. + */ + +import type { FeatureRequest, FeatureRequestPatch, FeatureRequestStatus } from "./types"; + +export const STATUSES: readonly FeatureRequestStatus[] = [ + "new", + "triaged", + "planned", + "done", + "declined", + "duplicate", +]; + +/** The edit form's values, as typed. */ +export interface Draft { + id: number; + status: FeatureRequestStatus; + note: string; + /** The original's number, or "" for none. */ + duplicateOf: string; +} + +export function draftOf(row: FeatureRequest): Draft { + return { + id: row.id, + status: row.status, + note: row.admin_note ?? "", + duplicateOf: row.duplicate_of === null ? "" : String(row.duplicate_of), + }; +} + +/** Why the draft cannot be sent, or null. */ +export function draftProblem(draft: Draft): string | null { + const duplicate = draft.duplicateOf.trim(); + if (duplicate !== "" && !/^\d+$/.test(duplicate)) return "Duplicate of must be a suggestion number."; + if (duplicate === String(draft.id)) return "A suggestion cannot be a duplicate of itself."; + return null; +} + +/** The fields that differ from the row, as the API takes them. */ +export function patchFor(row: FeatureRequest, draft: Draft): FeatureRequestPatch { + const patch: FeatureRequestPatch = {}; + if (draft.status !== row.status) patch.status = draft.status; + const note = draft.note.trim() === "" ? null : draft.note.trim(); + if (note !== row.admin_note) patch.admin_note = note; + const duplicate = draft.duplicateOf.trim() === "" ? null : Number(draft.duplicateOf.trim()); + if (duplicate !== row.duplicate_of) patch.duplicate_of = duplicate; + return patch; +} diff --git a/console/src/domain/types.ts b/console/src/domain/types.ts index 786c5d5..e3212e1 100644 --- a/console/src/domain/types.ts +++ b/console/src/domain/types.ts @@ -3,7 +3,9 @@ * * Two things are deliberately absent and their absence is the contract: * nothing here carries message, document or ask text, and nothing here - * carries a secret -- not even a masked one. If a field for either ever + * carries a secret -- not even a masked one. A feature request's text is not + * an exception: it is the person's own suggestion, given to the team on + * purpose, and never something they said in a channel. If a field for either ever * appears in this file, the console has started rendering something the API * promised never to send. * @@ -183,6 +185,49 @@ export interface SignedOut { end_session_url: string | null; } +/** Where the team is with a suggestion. The API's `RequestStatus`. */ +export type FeatureRequestStatus = "new" | "triaged" | "planned" | "done" | "declined" | "duplicate"; + +/** + * GET /api/feature-requests: one suggestion as the team sees it. + * + * `person` is their display name, never an account id; the console is not + * where anybody is contacted from. `same_text_elsewhere` counts the other + * people who suggested the same words. + */ +export interface FeatureRequest { + id: number; + text: string; + language: string | null; + status: FeatureRequestStatus; + admin_note: string | null; + duplicate_of: number | null; + source_kind: string; + person: string; + same_text_elsewhere: number; + created_at: string; + updated_at: string; + updated_by: string | null; +} + +export interface FeatureRequestPage { + items: FeatureRequest[]; + total: number; + page: number; + page_size: number; +} + +/** PATCH /api/feature-requests/{id}: only the fields that change. */ +export interface FeatureRequestPatch { + status?: FeatureRequestStatus; + admin_note?: string | null; + duplicate_of?: number | null; +} + +export interface FeatureRequestTriaged extends Changed { + request: FeatureRequest; +} + /** How GET /api/usage/summary groups its rows. */ export type UsageGroup = "person" | "feature" | "model" | "tool"; diff --git a/console/src/services/adminApi.ts b/console/src/services/adminApi.ts index 95a3dad..88ed28c 100644 --- a/console/src/services/adminApi.ts +++ b/console/src/services/adminApi.ts @@ -12,6 +12,10 @@ import type { Changed, Channel, ChannelAdded, + FeatureRequestPage, + FeatureRequestPatch, + FeatureRequestStatus, + FeatureRequestTriaged, FederationServer, OptOut, PersonQuestions, @@ -74,6 +78,14 @@ export const adminApi = { audit: () => send("GET", apiPath("audit")), + featureRequests: (status: FeatureRequestStatus | null, page: number) => { + const query = new URLSearchParams({ page: String(page) }); + if (status !== null) query.set("status", status); + return send("GET", `${apiPath("feature-requests")}?${query.toString()}`); + }, + /** Admin only; the API records every change with who made it. */ + triageFeatureRequest: (id: number, patch: FeatureRequestPatch) => + send("PATCH", apiPath("feature-requests", String(id)), patch), usageSummary: (window: UsageWindow, group: UsageGroup) => send("GET", withQuery(apiPath("usage", "summary"), { ...window, group })), /** diff --git a/console/src/services/http.ts b/console/src/services/http.ts index 52f7a15..8280de5 100644 --- a/console/src/services/http.ts +++ b/console/src/services/http.ts @@ -57,7 +57,7 @@ export class ApiError extends Error { /** What a refused credential says. One sentence for all four causes. */ export const REFUSED_MESSAGE = "That credential was refused."; -export type Method = "GET" | "POST" | "PUT" | "DELETE"; +export type Method = "GET" | "POST" | "PUT" | "PATCH" | "DELETE"; /** A cookie-mode request: the browser's session cookie, and no credential header. */ export function requestInit(method: Method, body?: unknown): RequestInit { diff --git a/console/src/test/stubApi.ts b/console/src/test/stubApi.ts index 39e86dd..beff069 100644 --- a/console/src/test/stubApi.ts +++ b/console/src/test/stubApi.ts @@ -171,6 +171,22 @@ function fixtures(): Json { revoked_at: null, }, ], + featureRequests: [ + { + id: 12, + text: "Tell me when someone mentions me", + language: "en", + status: "new", + admin_note: null, + duplicate_of: null, + source_kind: "command", + person: "Sam", + same_text_elsewhere: 2, + created_at: "2026-09-15T10:00:00+00:00", + updated_at: "2026-09-15T10:00:00+00:00", + updated_by: null, + }, + ], usage: usageFixtures(), questions: questionFixtures(), audit: [ @@ -316,6 +332,13 @@ function allowTool(state: Json, body: Json): [number, unknown] { type Reply = [status: number, payload: unknown]; +/** GET /api/feature-requests, with its status filter, as the API pages it. */ +function featurePage(state: Json, url: string): Json { + const status = new URL(url, "http://stub").searchParams.get("status"); + const items = (state.featureRequests as Json[]).filter((row) => status === null || row.status === status); + return { items, total: items.length, page: 1, page_size: 50 }; +} + /** One write route: the method, the path prefix, and what it does to the state. */ type WriteRoute = [method: string, prefix: string, handle: (state: Json, last: string, body: Json, path: string) => Reply]; @@ -365,6 +388,12 @@ const WRITES: WriteRoute[] = [ rows(state, "optouts").push({ person, since: "2026-09-16T10:00:00+00:00" }); return [200, { changed: `opt_out:${person}`, detail: "the exclusion is enforced in the database", removed: {} }]; }], + ["PATCH", "/api/feature-requests/", (state, last, body) => { + const request = rows(state, "featureRequests").find((row) => String(row.id) === last); + if (request === undefined) return [404, { error: "no such suggestion" }]; + Object.assign(request, body, { updated_by: "ana" }); + return [200, { changed: `feature_request.${last}`, request }]; + }], ["DELETE", "/api/optouts/", (state, _last, _body, path) => { const [, , , platform, id] = path.split("/"); state.optouts = rows(state, "optouts").filter((row) => row.person !== `${platform}:${id}`); @@ -531,6 +560,7 @@ export async function startStubApi(): Promise { "/api/optouts": state.optouts, "/api/tokens": state.tokens, "/api/audit": state.audit, + "/api/feature-requests": featurePage(state, request.url ?? ""), }; const found = reads[path]; reply(found === undefined ? 404 : 200, found ?? { error: "no such route" }); diff --git a/console/src/viewmodels/console.svelte.ts b/console/src/viewmodels/console.svelte.ts index 1eac95b..0c42953 100644 --- a/console/src/viewmodels/console.svelte.ts +++ b/console/src/viewmodels/console.svelte.ts @@ -12,6 +12,7 @@ import type { SessionPort } from "../services/session"; import type { SessionApi } from "../services/sessionApi"; import { AuditVM } from "./audit.svelte"; import { ChannelsVM } from "./channels.svelte"; +import { FeatureRequestsVM } from "./featureRequests.svelte"; import { FederationVM } from "./federation.svelte"; import { RetentionVM } from "./retention.svelte"; import { Router, type Route } from "./router.svelte"; @@ -64,6 +65,10 @@ export class ConsoleVM { return new TokensVM(this.#api); } + featureRequests(): FeatureRequestsVM { + return new FeatureRequestsVM(this.#api); + } + usage(): UsageVM { return new UsageVM(this.#api, () => this.session.canReadQuestions); } diff --git a/console/src/viewmodels/featureRequests.svelte.ts b/console/src/viewmodels/featureRequests.svelte.ts new file mode 100644 index 0000000..5925e2f --- /dev/null +++ b/console/src/viewmodels/featureRequests.svelte.ts @@ -0,0 +1,90 @@ +/** + * What people suggested, and its triage. + * + * Operators read the list; admins set a status, a note and a duplicate link, + * one row at a time. The server refuses an operator's change whatever this + * screen shows, and records every admin change with who made it. Authors who + * asked to hear about a status change are told by the bot, not from here. + */ + +import { draftOf, draftProblem, patchFor, type Draft } from "../domain/featureRequests"; +import type { FeatureRequest, FeatureRequestPage, FeatureRequestStatus } from "../domain/types"; +import type { AdminApi } from "../services/adminApi"; +import { Action } from "./action.svelte"; +import { Resource } from "./resource.svelte"; + +export type StatusFilter = FeatureRequestStatus | "all"; + +export const NOTHING_CHANGED = "Nothing changed."; + +type Api = Pick; + +export class FeatureRequestsVM { + filter: StatusFilter = $state("all"); + page = $state(1); + readonly requests: Resource; + readonly action = new Action(); + /** The row being edited, or null. */ + draft: Draft | null = $state(null); + readonly pages: number; + + readonly #api: Api; + + constructor(api: Api) { + this.#api = api; + this.requests = new Resource(() => + api.featureRequests(this.filter === "all" ? null : this.filter, this.page), + ); + this.pages = $derived(pageCount(this.requests.data)); + } + + load = (): Promise => this.requests.reload(); + + show = (filter: StatusFilter): void => { + this.filter = filter; + this.page = 1; + this.draft = null; + void this.load(); + }; + + turn = (by: number): void => { + const next = this.page + by; + if (next < 1 || next > this.pages) return; + this.page = next; + this.draft = null; + void this.load(); + }; + + edit = (row: FeatureRequest): void => { + this.action.clear(); + this.draft = draftOf(row); + }; + + cancel = (): void => { + this.draft = null; + }; + + save = async (row: FeatureRequest): Promise => { + const draft = this.draft; + if (draft === null || draft.id !== row.id) return; + await this.action.run(async () => { + const problem = draftProblem(draft); + if (problem !== null) throw new Error(problem); + const patch = patchFor(row, draft); + if (Object.keys(patch).length === 0) { + this.draft = null; + return NOTHING_CHANGED; + } + // Closed only once the server took it, so a refusal keeps what was typed. + await this.#api.triageFeatureRequest(row.id, patch); + if (this.draft === draft) this.draft = null; + void this.load(); + return `#${row.id} saved.`; + }); + }; +} + +function pageCount(page: FeatureRequestPage | null): number { + if (page === null || page.total === 0) return 1; + return Math.ceil(page.total / page.page_size); +} diff --git a/console/src/viewmodels/featureRequests.test.ts b/console/src/viewmodels/featureRequests.test.ts new file mode 100644 index 0000000..84748d0 --- /dev/null +++ b/console/src/viewmodels/featureRequests.test.ts @@ -0,0 +1,109 @@ +import { describe, expect, it } from "vitest"; + +import type { FeatureRequest, FeatureRequestPatch, FeatureRequestStatus } from "../domain/types"; +import { FeatureRequestsVM, NOTHING_CHANGED } from "./featureRequests.svelte"; + +const ROW: FeatureRequest = { + id: 12, + text: "dark mode", + language: "en", + status: "new", + admin_note: null, + duplicate_of: null, + source_kind: "command", + person: "Sam", + same_text_elsewhere: 0, + created_at: "2026-09-15T10:00:00+00:00", + updated_at: "2026-09-15T10:00:00+00:00", + updated_by: null, +}; + +function fakeApi(total = 1) { + const asked: [FeatureRequestStatus | null, number][] = []; + const patched: [number, FeatureRequestPatch][] = []; + const api = { + featureRequests: async (status: FeatureRequestStatus | null, page: number) => { + asked.push([status, page]); + return { items: [ROW], total, page, page_size: 50 }; + }, + triageFeatureRequest: async (id: number, patch: FeatureRequestPatch) => { + patched.push([id, patch]); + return { changed: `feature_request.${id}`, request: { ...ROW, ...patch } as FeatureRequest }; + }, + }; + return { api, asked, patched }; +} + +describe("FeatureRequestsVM", () => { + it("asks for one status from the first page", async () => { + const { api, asked } = fakeApi(); + const vm = new FeatureRequestsVM(api); + await vm.load(); + vm.show("planned"); + expect(asked).toEqual([ + [null, 1], + ["planned", 1], + ]); + }); + + it("turns pages within the total", async () => { + const { api, asked } = fakeApi(120); + const vm = new FeatureRequestsVM(api); + await vm.load(); + expect(vm.pages).toBe(3); + vm.turn(-1); + vm.turn(1); + expect(asked.map(([, page]) => page)).toEqual([1, 2]); + }); + + it("sends only the fields the admin changed", async () => { + const { api, patched } = fakeApi(); + const vm = new FeatureRequestsVM(api); + vm.edit(ROW); + if (vm.draft === null) throw new Error("no draft"); + vm.draft.status = "planned"; + await vm.save(ROW); + expect(patched).toEqual([[12, { status: "planned" }]]); + expect(vm.action.note).toBe("#12 saved."); + expect(vm.draft).toBeNull(); + }); + + it("sends nothing for an untouched draft", async () => { + const { api, patched } = fakeApi(); + const vm = new FeatureRequestsVM(api); + vm.edit(ROW); + await vm.save(ROW); + expect(patched).toEqual([]); + expect(vm.action.note).toBe(NOTHING_CHANGED); + }); + + it("keeps the draft and says why when it cannot be sent", async () => { + const { api, patched } = fakeApi(); + const vm = new FeatureRequestsVM(api); + vm.edit(ROW); + if (vm.draft === null) throw new Error("no draft"); + vm.draft.duplicateOf = "12"; + await vm.save(ROW); + expect(patched).toEqual([]); + expect(vm.action.error).toMatch(/itself/); + expect(vm.draft).not.toBeNull(); + }); + + it("keeps the typed draft when the server refuses the change", async () => { + const { api } = fakeApi(); + const refusing = { + ...api, + triageFeatureRequest: async () => { + throw new Error("duplicate_of names a suggestion that does not exist"); + }, + }; + const vm = new FeatureRequestsVM(refusing); + vm.edit(ROW); + if (vm.draft === null) throw new Error("no draft"); + vm.draft.status = "planned"; + vm.draft.note = "next quarter"; + await vm.save(ROW); + expect(vm.action.error).toMatch(/does not exist/); + expect(vm.draft).toMatchObject({ id: 12, status: "planned", note: "next quarter" }); + }); +}); diff --git a/console/src/views/routes.test.ts b/console/src/views/routes.test.ts index eae768b..d9eb42f 100644 --- a/console/src/views/routes.test.ts +++ b/console/src/views/routes.test.ts @@ -24,6 +24,7 @@ const READS: Record = { "/retention": ["/api/settings", "/api/optouts"], "/settings": ["/api/settings"], "/tokens": ["/api/tokens"], + "/feature-requests": ["/api/feature-requests"], // A person's questions are read on demand, and only offered to an admin // signed in as a person; the screen itself needs the summary alone. "/usage": ["/api/usage/summary"], diff --git a/console/src/views/routes.ts b/console/src/views/routes.ts index 8e16e05..fa5da03 100644 --- a/console/src/views/routes.ts +++ b/console/src/views/routes.ts @@ -13,6 +13,7 @@ import type { ConsoleVM } from "../viewmodels/console.svelte"; import type { Route } from "../viewmodels/router.svelte"; import AuditScreen from "./screens/AuditScreen.svelte"; import ChannelsScreen from "./screens/ChannelsScreen.svelte"; +import FeatureRequestsScreen from "./screens/FeatureRequestsScreen.svelte"; import FederationScreen from "./screens/FederationScreen.svelte"; import RetentionScreen from "./screens/RetentionScreen.svelte"; import SettingsScreen from "./screens/SettingsScreen.svelte"; @@ -29,6 +30,12 @@ export const ROUTES: readonly Route[] = [ { path: "/retention", title: "Retention", view: RetentionScreen, minRole: "operator" }, { path: "/settings", title: "Settings", view: SettingsScreen, minRole: "operator" }, { path: "/tokens", title: "Tokens", view: TokensScreen, minRole: "operator" }, + { + path: "/feature-requests", + title: "Feature requests", + view: FeatureRequestsScreen, + minRole: "operator", + }, { path: "/usage", title: "Usage", view: UsageScreen, minRole: "operator" }, { path: "/audit", title: "Audit", view: AuditScreen, minRole: "operator" }, ]; diff --git a/console/src/views/screens/FeatureRequestsScreen.svelte b/console/src/views/screens/FeatureRequestsScreen.svelte new file mode 100644 index 0000000..40847ba --- /dev/null +++ b/console/src/views/screens/FeatureRequestsScreen.svelte @@ -0,0 +1,145 @@ + + + + + {#snippet actions()} +
+ {#each FILTERS as status (status)} + + {/each} +
+ {/snippet} +

+ Authors who asked to be told get one direct message per status change. The note is for the + team and is never sent to them. +

+ + + {#snippet children(page)} + {#if page.items.length === 0} +

No {vm.filter === "all" ? "" : `${vm.filter} `}suggestions.

+ {:else} + + + + + + + + + + + + + + {#each page.items as row (row.id)} + + + + + {#if vm.draft !== null && vm.draft.id === row.id} + + + + + {:else} + + + + + {/if} + + {/each} + +
#SuggestionFromStatusNoteReceived
{row.id} + {row.text} + {#if row.same_text_elsewhere > 0} +

also suggested by {row.same_text_elsewhere} other(s)

+ {/if} + 		{row.person} + + + {formatTime(row.created_at)} + + + + {row.status} + {#if row.duplicate_of !== null} +

of #{row.duplicate_of}

+ {/if} + 		+ {#if row.admin_note}{row.admin_note}{:else}—{/if} + {formatTime(row.created_at)} + {#if app.session.canChange} + + {/if} +
+ {#if vm.pages > 1} +
+ + page {vm.page} of {vm.pages} + +
+ {/if} + {/if} + {/snippet} + + diff --git a/docs/admin-console.md b/docs/admin-console.md index e3fdb24..7bdec10 100644 --- a/docs/admin-console.md +++ b/docs/admin-console.md @@ -136,6 +136,7 @@ credential issue as an escalation. | Retention | How long anything is kept, and who has opted out. | | Settings | Every setting, its value, and **where that value came from**. | | Tokens | Review and revoke MCP credentials. Issuing one is not possible from here; see below. | +| Feature requests | What people suggested (`/suggest`), in their own words, with their display name and how many others suggested the same. Filter by status; admins triage a row (status, a note for the team, duplicate of #) and each changed field lands in the audit — the note as "set" or "empty", never its text. Authors who asked are told of a status change by the bot, not from here. | | Usage | Traced question runs, tokens, estimated cost, tool calls and voice time per person, feature, model and tool, for up to 90 days. An admin signed in with CyberdyneAuth can open one person's questions (their own words, paged, audited); everybody else sees counts only. See "Usage" below. | | Audit | Who changed what, when, from what to what. | @@ -435,10 +436,11 @@ rebuild the oracle the server refuses to be. ## Roles, and which route needs which The API has two console roles. **Operator** is read-only: status, settings, -federation, channels, opt-outs, tokens, the audit and the usage summary, and -nothing that changes them. **Admin** implies operator and is required for every write — a setting, -a federated server or tool, a channel, an opt-out (which purges), an MCP token -revocation. +federation, channels, opt-outs, tokens, feature requests, the audit and the +usage summary, and nothing that changes them. **Admin** implies operator and +is required for every write — a setting, a federated server or tool, a +channel, an opt-out (which purges), an MCP token revocation, a feature +request's triage. Which role a route needs is decided in one place, `ROUTE_ACCESS` in `src/chatmemory/admin/server.py`, with one explicit row per mounted diff --git a/docs/operations.md b/docs/operations.md index 3d8585b..9356379 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -1204,9 +1204,30 @@ surrounding conversation is stored. - **Five a day**: at most five accepted per person in any rolling 24 hours, enforced inside the insert while holding a lock on the person's row, so parallel submissions are counted one after another. +- **Triage**: the admin console's Feature requests screen lists every + suggestion, newest first, filterable by status (`new`, `triaged`, + `planned`, `done`, `declined`, `duplicate`), with the author's display name + and how many other people suggested the same text + (`GET /api/feature-requests`, operator). Admins set the status, a note for + the team and a duplicate-of link (`PATCH /api/feature-requests/{id}`, + admin; operators get 403). Each changed field is written to the change + record as `feature_request..status`, `.duplicate_of` or `.admin_note`, + with who made it; the status and link with their values before and after, + the note only as "set" or "empty", never its text. The suggestion's own + text cannot be edited. - **Status news is opt-in**: the acknowledgement asks whether to DM them when - the status changes; nothing is stored unless they press Yes. The sweep that - sends those DMs arrives with the admin triage screen. + the status changes; nothing is stored unless they press Yes. A sweep in the + bot process (every ten minutes, after the gateway connects) sends one DM + per status change, in the suggestion's language, naming the new status and + quoting their text; the admin note is never sent. A row is claimed before + its message is sent, so each change is announced once; a status changed and + changed back before the sweep sends nothing. Nobody is messaged who did not + press Yes, has opted out or erased their data (their rows are gone), has + turned `/notifications` off, or whose DMs are closed. A closed DM is + recorded like a notification's (`notification_preference.undeliverable_at`) + and the news waits until `/notifications on`; a transient Discord failure + is retried on the next sweep. The health endpoint reports + `suggestion_news.delivered` and `last_run_at`. - **Privacy**: nothing is written for an opted-out person, not even their name; an opt-out deletes a person's suggestions in the same transaction, and deleting the person cascades. The delete is in diff --git a/docs/roadmap.md b/docs/roadmap.md index 9971f16..3411f52 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -84,7 +84,8 @@ Last reviewed 2026-09-25. there is an answer (#45, #46). - **Feature requests** — `/suggest` and `/suggestions`, and six explicit suggestion forms in a message, confirmed with a button; no contact details, - idempotent, five a day, purged on opt-out. + idempotent, five a day, purged on opt-out. Triaged in the admin console + (audited); authors who opted in get one DM per status change. - **Admin console** — federation, channels, retention, opt-outs and tokens (#22, #23, #24). - **MCP interface** to the corpus, under the same permission rules; its diff --git a/openspec/changes/add-feature-requests/design.md b/openspec/changes/add-feature-requests/design.md index 11b2cfc..4f115ba 100644 --- a/openspec/changes/add-feature-requests/design.md +++ b/openspec/changes/add-feature-requests/design.md @@ -137,10 +137,12 @@ feature_request( The admin process cannot DM. A sweep in the bot process (every 10 minutes) selects rows where `notify_on_change` is true and `status <> notified_status`, -and sends a localised DM. It skips people whose -`notification_preference.undeliverable_at` is set, and then sets -`notified_status`. Before the admin triage endpoint exists, the sweep has -nothing to send. +claims them by setting `notified_status` to the status, and sends a localised +DM. It skips people who opted out, turned notifications off, or whose +`notification_preference.undeliverable_at` is set; their rows stay unclaimed. +An unsent message puts the claim back: a closed DM also records +`undeliverable_at` (cleared by `/notifications on`), and a transient failure is +retried on the next sweep. ## Risks / Trade-offs diff --git a/openspec/changes/add-feature-requests/tasks.md b/openspec/changes/add-feature-requests/tasks.md index 5f31705..85c09f9 100644 --- a/openspec/changes/add-feature-requests/tasks.md +++ b/openspec/changes/add-feature-requests/tasks.md @@ -20,8 +20,11 @@ ## 3. Triage and notification -- [ ] 3.1 `GET /api/feature-requests` (operator), `PATCH /api/feature-requests/{id}` (admin, audited); rows in the route-to-role table -- [ ] 3.2 Svelte `FeatureRequestsVM` + screen (filters by status, duplicate count) -- [ ] 3.3 Status-change DM sweep in the bot process (opt-in only) -- [ ] 3.4 Tests: operator PATCH -> 403; audit entry; DM sent once per status change; undeliverable skipped; no DM without opt-in -- [ ] 3.5 Docs: user-facing commands, admin console screen +- [x] 3.1 `GET /api/feature-requests` (operator), `PATCH /api/feature-requests/{id}` (admin, audited); rows in the route-to-role table + (one audit entry per changed field: `.status` and `.duplicate_of` with before/after, `.admin_note` as set/empty only; unknown fields, including `text`, are refused) +- [x] 3.2 Svelte `FeatureRequestsVM` + screen (filters by status, duplicate count) +- [x] 3.3 Status-change DM sweep in the bot process (opt-in only) + (`StatusNewsRunner`, every 10 minutes after the gateway connects; claims by advancing `notified_status` before sending and puts back an unsent claim; also skips people who turned `/notifications` off. No migration: 0030's columns suffice) +- [x] 3.4 Tests: operator PATCH -> 403; audit entry; DM sent once per status change; undeliverable skipped; no DM without opt-in + (plus opt-out, erasure and notifications-off send nothing; e2e through the assembled bot) +- [x] 3.5 Docs: user-facing commands, admin console screen diff --git a/src/chatmemory/adapters/discord/privacy.py b/src/chatmemory/adapters/discord/privacy.py index 1f1b39b..a7e8702 100644 --- a/src/chatmemory/adapters/discord/privacy.py +++ b/src/chatmemory/adapters/discord/privacy.py @@ -39,9 +39,9 @@ import structlog from chatmemory.adapters.discord.schedule_replies import outcome_label -from chatmemory.adapters.discord.suggestions import status_label from chatmemory.adapters.discord.views import RequesterOnlyView from chatmemory.app.fact_replies import label, shown_line +from chatmemory.app.feature_requests import status_label from chatmemory.app.language import Language from chatmemory.app.privacy import PrivacyReport, RetentionFacts from chatmemory.domain.chain import SUFFIX_CHARS diff --git a/src/chatmemory/adapters/discord/suggestions.py b/src/chatmemory/adapters/discord/suggestions.py index b6beae7..5c85c7a 100644 --- a/src/chatmemory/adapters/discord/suggestions.py +++ b/src/chatmemory/adapters/discord/suggestions.py @@ -27,13 +27,17 @@ import structlog from chatmemory.adapters.discord.views import RequesterOnlyView -from chatmemory.app.feature_requests import ContactKind, SubmitOutcome, SubmitResult +from chatmemory.app.feature_requests import ( + ContactKind, + SubmitOutcome, + SubmitResult, + status_label, +) from chatmemory.app.language import Language from chatmemory.ports.feature_requests import ( DEFAULT_DAILY_LIMIT, MAX_TEXT_CHARS, FeatureRequest, - RequestStatus, ) log = structlog.get_logger() @@ -153,26 +157,6 @@ }, } -_STATUSES: dict[Language, dict[RequestStatus, str]] = { - EN: { - RequestStatus.NEW: "new", - RequestStatus.TRIAGED: "triaged", - RequestStatus.PLANNED: "planned", - RequestStatus.DONE: "done", - RequestStatus.DECLINED: "declined", - RequestStatus.DUPLICATE: "duplicate", - }, - PT: { - RequestStatus.NEW: "nova", - RequestStatus.TRIAGED: "em análise", - RequestStatus.PLANNED: "planejada", - RequestStatus.DONE: "feita", - RequestStatus.DECLINED: "recusada", - RequestStatus.DUPLICATE: "duplicada", - }, -} - - def _lang(language: Language) -> Language: return PT if language is PT else EN @@ -181,10 +165,6 @@ def text(key: str, language: Language, **values: object) -> str: return _TEXT[key][_lang(language)].format(**values) -def status_label(status: RequestStatus, language: Language) -> str: - return _STATUSES[_lang(language)][status] - - _OUTCOME_KEYS = { SubmitOutcome.ALREADY_RECORDED: "already", SubmitOutcome.EMPTY: "empty", diff --git a/src/chatmemory/adapters/store/feature_requests_postgres.py b/src/chatmemory/adapters/store/feature_requests_postgres.py index 01f55b4..a0562b5 100644 --- a/src/chatmemory/adapters/store/feature_requests_postgres.py +++ b/src/chatmemory/adapters/store/feature_requests_postgres.py @@ -10,16 +10,25 @@ from sqlalchemy.ext.asyncio import AsyncConnection, AsyncEngine from chatmemory.adapters.store import feature_requests_sql as sql -from chatmemory.adapters.store import retention_sql +from chatmemory.adapters.store import notify_sql, retention_sql from chatmemory.adapters.store.memory_postgres import _person_id from chatmemory.domain.identity import PersonRef from chatmemory.ports.feature_requests import ( FeatureRequest, FeatureRequestStore, + FeatureRequestTriage, NewSuggestion, RequestStatus, + SourceKind, + StatusNews, + StatusNewsStore, StoreResult, StoreVerdict, + TriageChange, + TriageEntry, + TriagePage, + TriageRefusal, + TriageResult, ) @@ -142,7 +151,135 @@ def _insert_params( } +def _triage_entry(row: RowMapping) -> TriageEntry: + return TriageEntry( + id=int(row["id"]), + text=str(row["text"]), + language=row["language"], + status=RequestStatus(str(row["status"])), + admin_note=row["admin_note"], + duplicate_of=None if row["duplicate_of"] is None else int(row["duplicate_of"]), + source_kind=SourceKind(str(row["source_kind"])), + person_name=str(row["person_name"]), + same_text_elsewhere=int(row["same_text_elsewhere"]), + created_at=row["created_at"], + updated_at=row["updated_at"], + updated_by=row["updated_by"], + ) + + +def _applied(before: TriageEntry, change: TriageChange) -> dict[str, object]: + """The row's triage fields once `change` is applied to `before`.""" + return { + "status": (change.status or before.status).value, + "admin_note": change.admin_note if change.set_note else before.admin_note, + "duplicate_of": change.duplicate_of if change.set_duplicate else before.duplicate_of, + } + + +class PostgresFeatureRequestTriage: + """Implements `FeatureRequestTriage`, for the admin console.""" + + def __init__(self, engine: AsyncEngine) -> None: + self._engine = engine + + async def triage_page( + self, status: RequestStatus | None, *, offset: int, limit: int + ) -> TriagePage: + wanted = None if status is None else status.value + async with self._engine.connect() as conn: + rows = await conn.execute( + sql.TRIAGE_PAGE, {"status": wanted, "offset": offset, "limit": limit} + ) + entries = [_triage_entry(r) for r in rows.mappings()] + total = await conn.scalar(sql.TRIAGE_COUNT, {"status": wanted}) + return TriagePage(entries=entries, total=int(total or 0)) + + async def triage( + self, request_id: int, change: TriageChange, *, actor: str, now: datetime + ) -> TriageResult: + async with self._engine.begin() as conn: + before = await _locked_row(conn, request_id) + if before is None: + return TriageResult(refusal=TriageRefusal.NOT_FOUND) + target = change.duplicate_of if change.set_duplicate else None + if target is not None and not await conn.scalar( + sql.TRIAGE_EXISTS, {"request_id": target} + ): + return TriageResult(before=before, refusal=TriageRefusal.UNKNOWN_DUPLICATE) + updated = await conn.execute( + sql.TRIAGE_UPDATE, + { + **_applied(before, change), + "request_id": request_id, + "now": now, + "updated_by": actor, + }, + ) + # The opt-out trigger drops an update to an opted-out person's row; + # their rows are purged with the opt-out, so this is a race at most. + if not updated.rowcount: + return TriageResult(refusal=TriageRefusal.NOT_FOUND) + after = await _locked_row(conn, request_id) + return TriageResult(before=before, after=after) + + +async def _locked_row(conn: AsyncConnection, request_id: int) -> TriageEntry | None: + row = (await conn.execute(sql.TRIAGE_ROW, {"request_id": request_id})).mappings().first() + return None if row is None else _triage_entry(row) + + +class PostgresStatusNewsStore: + """Implements `StatusNewsStore`, for the bot's status sweep.""" + + def __init__(self, engine: AsyncEngine) -> None: + self._engine = engine + + async def claim_status_news(self, limit: int) -> Sequence[StatusNews]: + async with self._engine.begin() as conn: + rows = await conn.execute(sql.CLAIM_STATUS_NEWS, {"limit": limit}) + return [ + StatusNews( + request_id=int(r["id"]), + person=PersonRef(str(r["platform"]), int(r["platform_user_id"])), + status=RequestStatus(str(r["status"])), + previous=str(r["previous"]), + language=r["language"], + text=str(r["text"]), + ) + for r in rows.mappings() + ] + + async def release(self, news: StatusNews) -> None: + async with self._engine.begin() as conn: + await conn.execute( + sql.RELEASE_STATUS_NEWS, + { + "request_id": news.request_id, + "previous": news.previous, + "claimed": news.status.value, + }, + ) + + async def record_undeliverable(self, person: PersonRef, now: datetime) -> None: + # The notification queue's own record, so `/notifications on` reopens + # both, and their queued notifications are settled as it would do. + async with self._engine.begin() as conn: + person_id = await _known_person(conn, person) + if person_id is None: + return + params = {"person_id": person_id, "now": now} + await conn.execute(notify_sql.RECORD_UNDELIVERABLE, params) + await conn.execute(notify_sql.SETTLE_UNDELIVERABLE, params) + + if TYPE_CHECKING: # pragma: no cover - exists to fail type-checking, not to run def _conforms(store: PostgresFeatureRequestStore) -> FeatureRequestStore: return store + + def _triages(store: PostgresFeatureRequestTriage) -> FeatureRequestTriage: + return store + + def _news(store: PostgresStatusNewsStore) -> StatusNewsStore: + return store diff --git a/src/chatmemory/adapters/store/feature_requests_sql.py b/src/chatmemory/adapters/store/feature_requests_sql.py index 08679a7..8d847f0 100644 --- a/src/chatmemory/adapters/store/feature_requests_sql.py +++ b/src/chatmemory/adapters/store/feature_requests_sql.py @@ -79,3 +79,109 @@ notified_status = COALESCE(notified_status, status) WHERE id = :request_id AND person_id = :person_id """) + +# --- triage: the console's view of every suggestion --------------------------- +# +# These read every person's suggestions, which is the point: the console is +# where the team reads them. A suggestion is the person's own words, given to +# the team with a disclosure, and never channel content, so there is no +# channel predicate to bind. What is returned is the row itself, the person's +# current display name, and a count -- never the platform id. + +_TRIAGE_COLUMNS = """ + fr.id, fr.text, fr.language, fr.status, fr.admin_note, fr.duplicate_of, + fr.source_kind, fr.created_at, fr.updated_at, fr.updated_by, + p.display_name AS person_name, + (SELECT count(DISTINCT other.person_id) FROM feature_request other + WHERE other.normalized_hash = fr.normalized_hash + AND other.person_id <> fr.person_id) AS same_text_elsewhere +""" + +TRIAGE_PAGE = text(f""" +SELECT {_TRIAGE_COLUMNS} +FROM feature_request fr +JOIN person p ON p.id = fr.person_id +WHERE (CAST(:status AS text) IS NULL OR fr.status = CAST(:status AS text)) +ORDER BY fr.created_at DESC, fr.id DESC +LIMIT :limit OFFSET :offset +""") + +TRIAGE_COUNT = text(""" +SELECT count(*) FROM feature_request +WHERE (CAST(:status AS text) IS NULL OR status = CAST(:status AS text)) +""") + +#: Locked for the rest of the transaction, so the "before" an admin's change +#: is recorded against is the row the change was applied to. +TRIAGE_ROW = text(f""" +SELECT {_TRIAGE_COLUMNS} +FROM feature_request fr +JOIN person p ON p.id = fr.person_id +WHERE fr.id = :request_id +FOR UPDATE OF fr +""") + +TRIAGE_EXISTS = text(""" +SELECT EXISTS (SELECT 1 FROM feature_request WHERE id = :request_id) +""") + +TRIAGE_UPDATE = text(""" +UPDATE feature_request + SET status = :status, + admin_note = :admin_note, + duplicate_of = :duplicate_of, + updated_at = CAST(:now AS timestamptz), + updated_by = :updated_by +WHERE id = :request_id +""") + +# --- status news: the bot's sweep ------------------------------------------------ +# +# A row is news when its author said yes to "tell you when its status +# changes?" (`notify_on_change`, which also set `notified_status` to the +# status they were told about) and the status has moved since. Claiming +# advances `notified_status` first, so each change is sent once; an unsent +# message is put back with RELEASE_STATUS_NEWS. +# +# Skipped, and left unclaimed: people who opted out (their rows are purged +# anyway), people who turned notifications off, and people whose direct +# messages are known to be closed. The recipient is the platform account the +# suggestion was made from. + +CLAIM_STATUS_NEWS = text(""" +WITH due AS ( + SELECT fr.id, fr.notified_status AS previous, + ppi.platform, ppi.platform_user_id + FROM feature_request fr + JOIN LATERAL ( + SELECT platform, platform_user_id FROM person_platform_id + WHERE person_id = fr.person_id AND platform = fr.platform + ORDER BY platform_user_id + LIMIT 1 + ) ppi ON true + LEFT JOIN notification_preference pref ON pref.person_id = fr.person_id + WHERE fr.notify_on_change + AND fr.notified_status IS NOT NULL + AND fr.status <> fr.notified_status + AND NOT EXISTS ( + SELECT 1 FROM person_opt_out o WHERE o.person_id = fr.person_id + ) + AND (pref.person_id IS NULL + OR (pref.enabled AND pref.undeliverable_at IS NULL)) + ORDER BY fr.updated_at, fr.id + LIMIT :limit + FOR UPDATE OF fr SKIP LOCKED +) +UPDATE feature_request fr + SET notified_status = fr.status + FROM due + WHERE fr.id = due.id +RETURNING fr.id, fr.status, fr.language, fr.text, due.previous, + due.platform, due.platform_user_id +""") + +RELEASE_STATUS_NEWS = text(""" +UPDATE feature_request + SET notified_status = :previous +WHERE id = :request_id AND notified_status = :claimed +""") diff --git a/src/chatmemory/admin/handlers/__init__.py b/src/chatmemory/admin/handlers/__init__.py index ccd03b3..d46c1d6 100644 --- a/src/chatmemory/admin/handlers/__init__.py +++ b/src/chatmemory/admin/handlers/__init__.py @@ -13,6 +13,7 @@ optouts per-person withdrawal tokens MCP credentials, issued once and never recoverable changes the change record, read back + feature_requests what people suggested, and its triage Two rules hold in all of them, and neither is left to a reviewer to notice: @@ -20,6 +21,8 @@ document or an ask body, and `queries` is the only module that reaches the corpus at all -- where the statements are counts and timings. A test reflects over them and fails on a statement that names a content column. + A suggestion (`feature_requests`) is not corpus content: it is the + person's own words, given to the team on purpose with a disclosure. * **Nothing returns a credential.** The one exception is the MCP token a handler has just minted, which is returned exactly once because it exists nowhere else; a stored credential is a hash and no route returns even diff --git a/src/chatmemory/admin/handlers/feature_requests.py b/src/chatmemory/admin/handlers/feature_requests.py new file mode 100644 index 0000000..e5bf721 --- /dev/null +++ b/src/chatmemory/admin/handlers/feature_requests.py @@ -0,0 +1,214 @@ +"""Feature requests: reading what people suggested, and triaging it. + +`GET /api/feature-requests` lists suggestions to operators and admins, newest +first, optionally one status at a time. Each carries the person's display +name as it is now and how many other people suggested the same text, and +never their platform id: the team reads a suggestion, it does not contact its +author from here. + +`PATCH /api/feature-requests/{id}` is admin only (see `ROUTE_ACCESS`). It +sets the status, an admin note and a duplicate-of link, and every field that +changed is written to the change record with who changed it. Status and the +duplicate link are recorded with their values before and after; the note is +free text, so the record says only that it was set or cleared. + +A suggestion is the person's own words, given to the team on purpose with a +disclosure. It is not corpus content: no message, document or ask body is +reachable from here. The author hears about a status change only if they +asked to, and from the bot process (`app.feature_request_news`); this +process cannot message anybody. +""" + +from __future__ import annotations + +from collections.abc import Mapping +from typing import Any + +import structlog +from starlette.requests import Request +from starlette.responses import JSONResponse +from starlette.routing import Route + +from chatmemory.admin.audit import applied +from chatmemory.admin.auth import Actor +from chatmemory.admin.handlers.services import AdminServices +from chatmemory.admin.handlers.support import ( + Ok, + Refused, + acting_operator, + body_of, + moment, + path_int, +) +from chatmemory.app.clock import utc_now +from chatmemory.ports.feature_requests import ( + MAX_NOTE_CHARS, + RequestStatus, + TriageChange, + TriageEntry, + TriageRefusal, +) + +log = structlog.get_logger() + +PAGE_SIZE = 50 +FIELDS = frozenset({"status", "admin_note", "duplicate_of"}) +"""Everything a PATCH may name. The text is the person's and is not editable.""" + +STATUS_VALUES = ", ".join(s.value for s in RequestStatus) + + +def routes(services: AdminServices) -> list[Route]: + async def list_requests(request: Request) -> JSONResponse: + status = _status_filter(request.query_params.get("status")) + page = _page(request.query_params.get("page")) + found = await services.feature_requests.triage_page( + status, offset=(page - 1) * PAGE_SIZE, limit=PAGE_SIZE + ) + return JSONResponse( + { + "items": [_view(e) for e in found.entries], + "total": found.total, + "page": page, + "page_size": PAGE_SIZE, + } + ) + + async def triage(request: Request) -> JSONResponse: + operator = acting_operator() + request_id = path_int(request, "id") + change = _change(await body_of(request), request_id) + result = await services.feature_requests.triage( + request_id, change, actor=operator.name, now=utc_now() + ) + if result.refusal is TriageRefusal.UNKNOWN_DUPLICATE: + raise Refused("duplicate_of names a suggestion that does not exist") + if result.before is None or result.after is None: + raise Refused("no such suggestion", status=404) + await _record(services, operator, result.before, result.after) + log.info("admin.feature_request_triaged", request_id=request_id, operator=operator.name) + return Ok(changed=f"feature_request.{request_id}").response( + request=_view(result.after) + ) + + return [ + Route("/api/feature-requests", list_requests, methods=["GET"], name="feature_requests"), + Route( + "/api/feature-requests/{id}", + triage, + methods=["PATCH"], + name="triage_feature_request", + ), + ] + + +def _view(entry: TriageEntry) -> dict[str, Any]: + return { + "id": entry.id, + "text": entry.text, + "language": entry.language, + "status": entry.status.value, + "admin_note": entry.admin_note, + "duplicate_of": entry.duplicate_of, + "source_kind": entry.source_kind.value, + "person": entry.person_name, + "same_text_elsewhere": entry.same_text_elsewhere, + "created_at": moment(entry.created_at), + "updated_at": moment(entry.updated_at), + "updated_by": entry.updated_by, + } + + +def _status_filter(raw: str | None) -> RequestStatus | None: + if raw is None or not raw.strip(): + return None + return _status(raw.strip()) + + +def _status(raw: object) -> RequestStatus: + if isinstance(raw, str) and raw in RequestStatus._value2member_map_: + return RequestStatus(raw) + raise Refused(f"status must be one of: {STATUS_VALUES}") + + +def _page(raw: str | None) -> int: + if raw is None or not raw.strip().isdigit(): + return 1 + return max(int(raw), 1) + + +def _change(body: Mapping[str, Any], request_id: int) -> TriageChange: + unknown = set(body) - FIELDS + if unknown: + raise Refused(f"only {', '.join(sorted(FIELDS))} can be changed") + if not body: + raise Refused("nothing to change") + return TriageChange( + status=_status(body["status"]) if "status" in body else None, + set_note="admin_note" in body, + admin_note=_note(body.get("admin_note")), + set_duplicate="duplicate_of" in body, + duplicate_of=_duplicate(body.get("duplicate_of"), request_id), + ) + + +def _note(raw: object) -> str | None: + if raw is None: + return None + if not isinstance(raw, str): + raise Refused("admin_note must be a string or null") + note = raw.strip() + if len(note) > MAX_NOTE_CHARS: + raise Refused(f"admin_note is limited to {MAX_NOTE_CHARS} characters") + return note or None + + +def _duplicate(raw: object, request_id: int) -> int | None: + if raw is None: + return None + if isinstance(raw, bool) or not isinstance(raw, int | str): + raise Refused("duplicate_of must be a suggestion number or null") + text = str(raw).strip() + if not text.isdigit(): + raise Refused("duplicate_of must be a suggestion number or null") + target = int(text) + if target == request_id: + raise Refused("a suggestion cannot be a duplicate of itself") + return target + + +async def _record( + services: AdminServices, operator: Actor, before: TriageEntry, after: TriageEntry +) -> None: + """One entry per field that changed. The note's text is never recorded.""" + setting = f"feature_request.{after.id}" + if before.status is not after.status: + await services.changes.record( + applied(operator, f"{setting}.status", before.status.value, after.status.value) + ) + if before.duplicate_of != after.duplicate_of: + await services.changes.record( + applied( + operator, + f"{setting}.duplicate_of", + _number(before.duplicate_of), + _number(after.duplicate_of), + ) + ) + if before.admin_note != after.admin_note: + await services.changes.record( + applied( + operator, + f"{setting}.admin_note", + _presence(before.admin_note), + _presence(after.admin_note), + ) + ) + + +def _number(value: int | None) -> str | None: + return None if value is None else f"#{value}" + + +def _presence(note: str | None) -> str: + return "set" if note else "empty" diff --git a/src/chatmemory/admin/handlers/services.py b/src/chatmemory/admin/handlers/services.py index d642d25..f800c2c 100644 --- a/src/chatmemory/admin/handlers/services.py +++ b/src/chatmemory/admin/handlers/services.py @@ -28,6 +28,7 @@ from chatmemory.app.optout import OptOutService from chatmemory.app.tokens import TokenDirectory from chatmemory.app.usage import UsageService +from chatmemory.ports.feature_requests import FeatureRequestTriage ServerProbe = Callable[[ServerConfig], Awaitable[ServerDiscovery]] """Ask one federated server what it offers, without joining it to anything. @@ -74,6 +75,9 @@ class AdminServices: mcp_tokens: TokenDirectory #: Reachability and offered tools, asked of a server before it is stored. probe: ServerProbe + #: What people suggested, and the admin's triage of it. A suggestion is + #: the person's own words given to the team, never corpus content. + feature_requests: FeatureRequestTriage #: Usage and cost, read live from the trace store with exclusions applied. #: The only port here that returns content: a person's own questions, for #: an admin signed in as a person (`admin_oidc`), audited. diff --git a/src/chatmemory/admin/server.py b/src/chatmemory/admin/server.py index e5ddee0..445b014 100644 --- a/src/chatmemory/admin/server.py +++ b/src/chatmemory/admin/server.py @@ -51,6 +51,7 @@ from chatmemory.admin.handlers import ( changes, channels, + feature_requests, federation, optouts, settings, @@ -111,6 +112,7 @@ def api_routes(services: AdminServices) -> list[Route]: *optouts.routes(services), *tokens.routes(services), *changes.routes(services), + *feature_requests.routes(services), *usage.routes(services), ] @@ -161,6 +163,9 @@ def api_routes(services: AdminServices) -> list[Route]: # MCP credentials: review and revoke only. ("GET", "/api/tokens"): _OPERATOR, ("DELETE", "/api/tokens/{id}"): _ADMIN, + # Feature requests: operators read them, admins triage (audited). + ("GET", "/api/feature-requests"): _OPERATOR, + ("PATCH", "/api/feature-requests/{id}"): _ADMIN, # Usage: counts for operators; a person's own questions only for an admin # signed in as a person, never for a token (see handlers/usage.py). ("GET", "/api/usage/summary"): _OPERATOR, diff --git a/src/chatmemory/app/feature_request_news.py b/src/chatmemory/app/feature_request_news.py new file mode 100644 index 0000000..184bb4e --- /dev/null +++ b/src/chatmemory/app/feature_request_news.py @@ -0,0 +1,127 @@ +"""Telling a person their suggestion's status changed, when they asked to be told. + +The admin console changes a status; it cannot message anybody. This sweep runs +in the bot process, which can, and sends one direct message per status change +to the author of a suggestion who answered Yes to "tell you when its status +changes?". Nobody else is ever messaged: + +* **Opt-in only.** A row is news only when `notify_on_change` is set, and + answering Yes starts `notified_status` at the status they already know, + so the first message is about the first change. +* **Once per change.** A row is claimed by advancing `notified_status` to + the status it is about to announce, before the message is sent. The next + sweep finds nothing new until an admin changes the status again. +* **Never where they said no.** Opted-out people, people who turned + notifications off and people whose direct messages are closed are not + claimed. Opting out and deleting everything purge the rows themselves. + +An unsent message is put back: a transient failure is retried on the next +sweep, and a closed direct message records the person as undeliverable (the +same record `/notifications` keeps), which leaves their news unclaimed until +they turn notifications back on. +""" + +from __future__ import annotations + +import structlog + +from chatmemory.app.clock import Clock, utc_now +from chatmemory.app.feature_requests import status_label +from chatmemory.app.language import Language +from chatmemory.app.schedules import TaskMessenger +from chatmemory.ports.feature_requests import StatusNews, StatusNewsStore +from chatmemory.ports.notifications import DeliveryResult + +log = structlog.get_logger() + +SWEEP_INTERVAL_SECONDS = 600.0 +"""How often the bot looks for status changes to announce.""" + +BATCH = 50 +"""Messages per sweep at most; the rest wait for the next one.""" + +QUOTED_TEXT_CHARS = 200 +"""How much of their own suggestion the message quotes back to them.""" + +_MESSAGE = { + Language.ENGLISH: ( + "Your suggestion **#{id}** is now **{status}**.\n> {text}\n" + "`/suggestions` lists all of yours." + ), + Language.PORTUGUESE: ( + "Sua sugestão **#{id}** agora está: **{status}**.\n> {text}\n" + "`/suggestions` mostra todas as suas." + ), +} + + +def _language(code: str | None) -> Language: + return Language.PORTUGUESE if code == "pt" else Language.ENGLISH + + +def _clip(text: str) -> str: + one_line = " ".join(text.split()) + if len(one_line) <= QUOTED_TEXT_CHARS: + return one_line + return one_line[: QUOTED_TEXT_CHARS - 1] + "…" + + +def status_message(news: StatusNews) -> str: + """The direct message, in the suggestion's own language.""" + language = _language(news.language) + return _MESSAGE[language].format( + id=news.request_id, + status=status_label(news.status, language), + text=_clip(news.text), + ) + + +class StatusNewsRunner: + """One sweep: claim the changes to announce, send each, put back the unsent.""" + + def __init__( + self, + store: StatusNewsStore, + messenger: TaskMessenger, + clock: Clock = utc_now, + batch: int = BATCH, + ) -> None: + self._store = store + self._messenger = messenger + self._clock = clock + self._batch = batch + + async def run_due(self) -> int: + """Send what is due; how many messages arrived. + + One item's failure never abandons the rest of the batch: every claim + is either sent or put back, so no status change is lost for good. + """ + sent = 0 + for news in await self._store.claim_status_news(self._batch): + if await self._announce(news): + sent += 1 + return sent + + async def _announce(self, news: StatusNews) -> bool: + try: + result = await self._messenger.deliver( + news.person, news.request_id, status_message(news) + ) + except Exception: # an unexpected failure is a transient one + log.exception("suggestions.status_news_deliver_failed", request_id=news.request_id) + result = DeliveryResult.FAILED + if result is DeliveryResult.SENT: + return True + try: + await self._store.release(news) + if result is DeliveryResult.CLOSED: + await self._store.record_undeliverable(news.person, self._clock()) + except Exception: + log.exception("suggestions.status_news_release_failed", request_id=news.request_id) + log.info( + "suggestions.status_news_not_sent", + request_id=news.request_id, + result=str(result), + ) + return False diff --git a/src/chatmemory/app/feature_requests.py b/src/chatmemory/app/feature_requests.py index 4e12667..e6fdbed 100644 --- a/src/chatmemory/app/feature_requests.py +++ b/src/chatmemory/app/feature_requests.py @@ -39,6 +39,7 @@ FeatureRequest, FeatureRequestStore, NewSuggestion, + RequestStatus, StoreVerdict, SuggestionSource, ) @@ -46,6 +47,31 @@ LISTING_LIMIT = 20 """How many of their own suggestions `/suggestions` shows, newest first.""" +_STATUS_LABELS: dict[Language, dict[RequestStatus, str]] = { + Language.ENGLISH: { + RequestStatus.NEW: "new", + RequestStatus.TRIAGED: "triaged", + RequestStatus.PLANNED: "planned", + RequestStatus.DONE: "done", + RequestStatus.DECLINED: "declined", + RequestStatus.DUPLICATE: "duplicate", + }, + Language.PORTUGUESE: { + RequestStatus.NEW: "nova", + RequestStatus.TRIAGED: "em análise", + RequestStatus.PLANNED: "planejada", + RequestStatus.DONE: "feita", + RequestStatus.DECLINED: "recusada", + RequestStatus.DUPLICATE: "duplicada", + }, +} + + +def status_label(status: RequestStatus, language: Language) -> str: + """A status as the person reads it: Portuguese, else English.""" + shown = language if language is Language.PORTUGUESE else Language.ENGLISH + return _STATUS_LABELS[shown][status] + class ContactKind(StrEnum): """What kind of contact detail made a suggestion unstorable.""" diff --git a/src/chatmemory/composition.py b/src/chatmemory/composition.py index 84455d0..98271ff 100644 --- a/src/chatmemory/composition.py +++ b/src/chatmemory/composition.py @@ -114,7 +114,10 @@ from chatmemory.adapters.store.decisions_postgres import PostgresDecisionStore from chatmemory.adapters.store.erasure_postgres import PostgresErasureStore from chatmemory.adapters.store.facts_postgres import PostgresFactStore -from chatmemory.adapters.store.feature_requests_postgres import PostgresFeatureRequestStore +from chatmemory.adapters.store.feature_requests_postgres import ( + PostgresFeatureRequestStore, + PostgresStatusNewsStore, +) from chatmemory.adapters.store.media_postgres import PostgresVoiceLedger from chatmemory.adapters.store.memory_postgres import PostgresMemoryStore from chatmemory.adapters.store.notify_postgres import PostgresNotificationQueue @@ -169,6 +172,7 @@ from chatmemory.app.decisions.model import DecisionPolicy from chatmemory.app.erasure import ErasureService from chatmemory.app.facts import PersonalFactsService +from chatmemory.app.feature_request_news import StatusNewsRunner from chatmemory.app.feature_requests import FeatureRequestService from chatmemory.app.limits import RateLimiter from chatmemory.app.notifications import ( @@ -1103,6 +1107,17 @@ def build_feature_requests(engine: AsyncEngine, clock: Clock = utc_now) -> Featu return FeatureRequestService(PostgresFeatureRequestStore(engine), clock=clock) +def build_feature_request_news( + engine: AsyncEngine, messenger: TaskMessenger, clock: Clock = utc_now +) -> StatusNewsRunner: + """The sweep that tells an author their suggestion's status changed. + + Only people who answered Yes are ever messaged, so, like `/suggest`, it is + not behind a setting. + """ + return StatusNewsRunner(PostgresStatusNewsStore(engine), messenger, clock=clock) + + def build_privacy( settings: Settings, engine: AsyncEngine, diff --git a/src/chatmemory/entrypoints/admin.py b/src/chatmemory/entrypoints/admin.py index ce3b2f7..852d507 100644 --- a/src/chatmemory/entrypoints/admin.py +++ b/src/chatmemory/entrypoints/admin.py @@ -67,6 +67,7 @@ PostgresSessionStore, ) from chatmemory.adapters.store.config_postgres import PostgresConfigurationStore +from chatmemory.adapters.store.feature_requests_postgres import PostgresFeatureRequestTriage from chatmemory.adapters.store.retention_sql import PostgresRetentionStore from chatmemory.adapters.store.trace_postgres import PostgresTraceIndex from chatmemory.adapters.store.usage_postgres import PostgresUsageDirectory @@ -338,6 +339,7 @@ def build( ), mcp_tokens=PostgresTokenStore(engine), probe=make_probe(), + feature_requests=PostgresFeatureRequestTriage(engine), usage=UsageService( usage_source(environ, transport), PostgresUsageDirectory(engine), diff --git a/src/chatmemory/entrypoints/bot.py b/src/chatmemory/entrypoints/bot.py index 7a75632..2f71a91 100644 --- a/src/chatmemory/entrypoints/bot.py +++ b/src/chatmemory/entrypoints/bot.py @@ -86,6 +86,11 @@ without `ALERTS_ENABLED` (and an Infura key) `BotGraph.alerts` is None and nothing starts. +Suggestion status messages take the same messenger: `build_bot` builds a +`StatusNewsRunner` over the answer stack's engine beside `/suggest`, and +`main` runs `suggestion_news_loop` every ten minutes. The admin console +changes a status; this is the process that tells the authors who asked. + Voice questions take the same edges: `assemble` -> `build_voice_questions(settings, engine, edges.http_transport, edges.clock)` -> `build_bot(voice=...)` -> `CyberFriendClient.attach_voice`. The CDN @@ -142,6 +147,8 @@ from chatmemory.app.configuration import ConfigurationEditor from chatmemory.app.conversation import Conversations from chatmemory.app.facts import PersonalFactsService +from chatmemory.app.feature_request_news import SWEEP_INTERVAL_SECONDS as SUGGESTION_SWEEP_SECONDS +from chatmemory.app.feature_request_news import StatusNewsRunner from chatmemory.app.indexing import ChannelPurge, IndexingService from chatmemory.app.notifications import NotificationDelivery from chatmemory.app.reasoning.contract import RunTracer @@ -162,6 +169,7 @@ build_catch_up, build_channel_listing, build_conversations, + build_feature_request_news, build_feature_requests, build_live_scope, build_notification_delivery, @@ -242,6 +250,9 @@ class BotGraph: #: The position-alert sweep. None unless `ALERTS_ENABLED` with an Infura #: key; alerts are created by asking and confirming, under the same switch. alerts: AlertRunner | None = None + #: The suggestion status sweep: one direct message per status change to + #: authors who asked for it. None when no engine was handed in. + suggestion_news: StatusNewsRunner | None = None #: The queue drain. None when no engine was handed in, or when an #: operator has switched notifications off -- in which case nothing in #: this process sends anything, which is the safe half. @@ -375,10 +386,17 @@ def provider() -> _Guild | None: runner = build_task_runner( settings, notifications, asks, DiscordTaskMessenger(client.fetch_user) ) + suggestion_news = None if notifications is not None: # `/suggest` and `/suggestions`. Without it both say suggestions cannot # be taken here. client.attach_feature_requests(build_feature_requests(notifications, clock)) + # And the other half: the console changes a status and cannot message + # anybody, so this process tells the authors who asked to be told. + # Without it the Yes under an acknowledgement promises nothing. + suggestion_news = build_feature_request_news( + notifications, DiscordTaskMessenger(client.fetch_user, prefix=""), clock + ) # `/privacy`. Archive coverage is the `/channels` listing, so a channel # the person cannot read is neither named nor counted; without a # listing no channel is. Without the engine the command says it cannot @@ -406,6 +424,7 @@ def provider() -> _Guild | None: notifications=delivery, tasks=runner, alerts=alerts, + suggestion_news=suggestion_news, ) @@ -612,6 +631,29 @@ async def alert_loop( await asyncio.sleep(interval) +async def suggestion_news_loop( + runner: StatusNewsRunner, + state: HealthState, + interval: float = SUGGESTION_SWEEP_SECONDS, + ready: asyncio.Event | None = None, +) -> None: + """Tell authors who asked that their suggestion's status changed. + + Waits for the gateway, since a direct message is all it produces. Absorbs + everything like the other sweeps: a claim whose message was not sent is + put back, so a failed pass is retried by the next one. + """ + if ready is not None: + await ready.wait() + while True: + try: + sent = await runner.run_due() + state.details["suggestion_news"] = {"delivered": sent, "last_run_at": time.time()} + except Exception: + log.exception("suggestions.status_sweep_failed") + await asyncio.sleep(interval) + + async def run_beside_scope( work: Coroutine[Any, Any, None], scope: LiveScope, @@ -814,6 +856,10 @@ async def on_ready() -> None: clock=process.edges.clock, ) ) + # Suggestion status messages, for the same reason: the console changes a + # status, and only this process can tell the author. + if graph.suggestion_news is not None: + drains.append(suggestion_news_loop(graph.suggestion_news, state, ready=gateway_ready)) await run_beside_scope( client.start(settings.discord_token.get_secret_value()), scope, state, drains ) diff --git a/src/chatmemory/ports/feature_requests.py b/src/chatmemory/ports/feature_requests.py index 858a043..0ea623b 100644 --- a/src/chatmemory/ports/feature_requests.py +++ b/src/chatmemory/ports/feature_requests.py @@ -109,3 +109,125 @@ async def for_person(self, person: PersonRef, limit: int) -> Sequence[FeatureReq async def set_notify(self, person: PersonRef, request_id: int, notify: bool) -> bool: """Whether to message them on a status change; False when not theirs.""" ... + + +# --- triage: the team's side ------------------------------------------------------- + +MAX_NOTE_CHARS = 2000 +"""The longest admin note the console keeps.""" + + +@dataclass(frozen=True, slots=True) +class TriageEntry: + """One suggestion as the team sees it in the console. + + The person's name is read from their person row when the list is asked + for, so a rename shows and nothing about them is copied onto the row. + `same_text_elsewhere` counts the *other* people who suggested the same + normalised text. + """ + + id: int + text: str + language: str | None + status: RequestStatus + admin_note: str | None + duplicate_of: int | None + source_kind: SourceKind + person_name: str + same_text_elsewhere: int + created_at: datetime + updated_at: datetime + updated_by: str | None + + +@dataclass(frozen=True, slots=True) +class TriagePage: + entries: Sequence[TriageEntry] + total: int + """How many suggestions the filter matches, across every page.""" + + +@dataclass(frozen=True, slots=True) +class TriageChange: + """What an admin asked to change. A field left None is left as it is. + + The note and the duplicate link can be cleared, so each has its own flag + saying it was given: `set_note` with `admin_note=None` clears the note. + """ + + status: RequestStatus | None = None + set_note: bool = False + admin_note: str | None = None + set_duplicate: bool = False + duplicate_of: int | None = None + + @property + def empty(self) -> bool: + return self.status is None and not self.set_note and not self.set_duplicate + + +class TriageRefusal(StrEnum): + NOT_FOUND = "not_found" + UNKNOWN_DUPLICATE = "unknown_duplicate" + """`duplicate_of` names a suggestion that does not exist.""" + + +@dataclass(frozen=True, slots=True) +class TriageResult: + before: TriageEntry | None = None + after: TriageEntry | None = None + refusal: TriageRefusal | None = None + + +class FeatureRequestTriage(Protocol): + """The console's port: every suggestion, and the admin's changes to one.""" + + async def triage_page( + self, status: RequestStatus | None, *, offset: int, limit: int + ) -> TriagePage: + """Newest first, optionally one status only.""" + ... + + async def triage( + self, request_id: int, change: TriageChange, *, actor: str, now: datetime + ) -> TriageResult: + """Apply `change`, recording who made it; the row before and after.""" + ... + + +# --- status news: telling the author ------------------------------------------------ + + +@dataclass(frozen=True, slots=True) +class StatusNews: + """A status change the author asked to hear about and has not been told. + + Claimed means `notified_status` already holds `status`; `previous` is what + it held, so an unsent message can be put back. + """ + + request_id: int + person: PersonRef + status: RequestStatus + previous: str + language: str | None + text: str + + +class StatusNewsStore(Protocol): + async def claim_status_news(self, limit: int) -> Sequence[StatusNews]: + """Claim changes to tell, for people who opted in to hearing them. + + Skips people who opted out, turned notifications off, or whose direct + messages are known to be closed; their rows stay unclaimed. + """ + ... + + async def release(self, news: StatusNews) -> None: + """Put back a claim whose message was not sent, unless it moved on.""" + ... + + async def record_undeliverable(self, person: PersonRef, now: datetime) -> None: + """Their direct messages are closed: stop trying until they reopen them.""" + ... diff --git a/tests/e2e/test_suggestions.py b/tests/e2e/test_suggestions.py index 896e512..73e6813 100644 --- a/tests/e2e/test_suggestions.py +++ b/tests/e2e/test_suggestions.py @@ -11,15 +11,18 @@ from __future__ import annotations +from datetime import UTC, datetime from typing import Any from sqlalchemy import text from chatmemory.adapters.discord.bot import PLATFORM from chatmemory.adapters.discord.suggestions import PROPOSAL_WINDOW_SECONDS +from chatmemory.adapters.store.feature_requests_postgres import PostgresFeatureRequestTriage from chatmemory.adapters.store.retention_sql import PostgresRetentionStore from chatmemory.app.optout import OptOutService from chatmemory.domain.identity import PersonRef +from chatmemory.ports.feature_requests import RequestStatus, TriageChange from tests.e2e.harness.conversation import Conversation, E2EBot, Turn from tests.e2e.harness.discord_wire import Sent from tests.e2e.harness.process import GENERAL, GUILD_ID @@ -27,6 +30,8 @@ from tests.e2e.test_portfolio import BASE_HOST, base_chain, save_wallet from tests.e2e.test_preferred_currency import bitcoin, market_bot # noqa: F401 +NOW = datetime(2026, 9, 26, 12, tzinfo=UTC) + async def suggestion_rows(bot: E2EBot) -> list[Any]: async with bot.engine.connect() as conn: @@ -78,6 +83,33 @@ async def test_suggest_then_list_it_and_opt_into_status_news(bot: E2EBot) -> Non assert "Show a weekly digest" not in (await bot.dm(ana).slash("suggestions")).text +async def test_a_status_change_is_told_once_to_the_author_who_asked(bot: E2EBot) -> None: + """The console changes the status; the bot's sweep sends one DM, and none + to somebody who did not say Yes.""" + leo, ana = bot.person("Leo"), bot.person("Ana") + asked = await bot.channel("general", leo).slash("suggest", text="Dark mode for the digest") + [ack] = asked.sent + await bot.channel("general", leo).press(ack, "Yes") + await bot.channel("general", ana).slash("suggest", text="Voice replies") + mine, theirs = [row.id for row in await suggestion_rows(bot)] + triage = PostgresFeatureRequestTriage(bot.engine) + for request_id in (mine, theirs): + await triage.triage( + request_id, TriageChange(status=RequestStatus.PLANNED), actor="ana", now=NOW + ) + runner = bot.process.graph.suggestion_news + assert runner is not None + + swept = await bot.turn(runner.run_due) + again = await bot.turn(runner.run_due) + + [message] = swept.sent + assert message.via == "dm" + assert message.content.startswith(f"Your suggestion **#{mine}** is now **planned**.") + assert "> Dark mode for the digest" in message.content + assert again.sent == () + + async def test_the_same_suggestion_twice_is_one_record(bot: E2EBot) -> None: dm = bot.dm(bot.person("Leo")) await dm.slash("suggest", text="Dark mode, please!") diff --git a/tests/integration/test_admin_api.py b/tests/integration/test_admin_api.py index a0028c7..ed87e6b 100644 --- a/tests/integration/test_admin_api.py +++ b/tests/integration/test_admin_api.py @@ -27,10 +27,13 @@ from sqlalchemy import text from sqlalchemy.ext.asyncio import AsyncEngine +from chatmemory.adapters.store.feature_requests_postgres import PostgresFeatureRequestStore from chatmemory.admin.auth import Operator +from chatmemory.app.feature_requests import FeatureRequestService from chatmemory.domain.identity import PersonRef from chatmemory.entrypoints import admin from chatmemory.mcp.auth import PostgresTokenStore +from chatmemory.ports.feature_requests import SourceKind, SuggestionSource from tests.integration.conftest import DB_URL ANA = Operator("ana") @@ -55,6 +58,7 @@ "/api/optouts", "/api/tokens", "/api/audit", + "/api/feature-requests", ] @@ -439,3 +443,44 @@ async def test_a_channel_added_through_the_api_is_in_force_for_the_agent( assert added.status_code == 200 stored = {s.key: s.raw for s in await console.services.editor.store.load()} assert sorted(stored["indexed_channel_ids"].split()) == ["100", "200", "900"] + + +async def test_a_triage_through_the_api_lands_in_the_audit_without_the_note( + console: admin.ConsoleProcess, +) -> None: + submitted = await FeatureRequestService(PostgresFeatureRequestStore(console.engine)).submit( + PersonRef("discord", 42), + "dark mode for the console", + SuggestionSource(SourceKind.COMMAND, "discord"), + display_name="Sam", + ) + token = await credential(console) + + async with client(console, token) as http: + listed = (await http.get("/api/feature-requests?status=new")).json() + response = await http.patch( + f"/api/feature-requests/{submitted.request_id}", + json={"status": "planned", "admin_note": "Q4, after the Svelte work"}, + ) + + assert [(i["person"], i["text"]) for i in listed["items"]] == [ + ("Sam", "dark mode for the console") + ] + assert response.status_code == 200 + async with console.engine.connect() as conn: + rows = await conn.execute( + text( + "SELECT operator, setting, before_value, after_value FROM config_audit " + "WHERE setting LIKE 'feature_request.%' ORDER BY setting" + ) + ) + audited = [tuple(r) for r in rows] + stored = await conn.scalar( + text("SELECT updated_by FROM feature_request WHERE id = :i"), + {"i": submitted.request_id}, + ) + assert audited == [ + ("ana", f"feature_request.{submitted.request_id}.admin_note", "empty", "set"), + ("ana", f"feature_request.{submitted.request_id}.status", "new", "planned"), + ] + assert stored == "ana" diff --git a/tests/integration/test_feature_request_triage.py b/tests/integration/test_feature_request_triage.py new file mode 100644 index 0000000..3a00cf8 --- /dev/null +++ b/tests/integration/test_feature_request_triage.py @@ -0,0 +1,357 @@ +"""Triage and status news against real Postgres. + +The console's list and its audited change, and the bot's sweep: a message is +due only for an author who said Yes, once per status change, and never for +somebody who opted out, erased their data, turned notifications off or +closed their direct messages. +""" + +from __future__ import annotations + +from datetime import UTC, datetime, timedelta + +import pytest +from sqlalchemy import text +from sqlalchemy.ext.asyncio import AsyncEngine + +from chatmemory.adapters.store.feature_requests_postgres import ( + PostgresFeatureRequestStore, + PostgresFeatureRequestTriage, + PostgresStatusNewsStore, +) +from chatmemory.adapters.store.notify_postgres import PostgresNotificationQueue +from chatmemory.adapters.store.retention_sql import PostgresRetentionStore +from chatmemory.app.feature_request_news import StatusNewsRunner +from chatmemory.app.feature_requests import FeatureRequestService +from chatmemory.app.optout import OptOutService +from chatmemory.domain.identity import PersonRef +from chatmemory.ports.feature_requests import ( + RequestStatus, + SourceKind, + SuggestionSource, + TriageChange, + TriageRefusal, +) +from chatmemory.ports.notifications import DeliveryResult + +pytestmark = pytest.mark.asyncio + +PLATFORM = "discord" +NOW = datetime(2026, 9, 26, 12, 0, tzinfo=UTC) +LEO = PersonRef(PLATFORM, 7) +ANA = PersonRef(PLATFORM, 9) +COMMAND = SuggestionSource(SourceKind.COMMAND, PLATFORM, guild_id=1, channel_id=100) + + +class Messenger: + def __init__(self, result: DeliveryResult = DeliveryResult.SENT) -> None: + self.result = result + self.sent: list[tuple[PersonRef, int, str]] = [] + + async def deliver(self, person: PersonRef, task_id: int, text: str) -> DeliveryResult: + if self.result is DeliveryResult.SENT: + self.sent.append((person, task_id, text)) + return self.result + + +async def suggest( + engine: AsyncEngine, person: PersonRef, words: str, *, notify: bool, name: str = "" +) -> int: + service = FeatureRequestService(PostgresFeatureRequestStore(engine), clock=lambda: NOW) + result = await service.submit(person, words, COMMAND, display_name=name) + assert result.request_id is not None + if notify: + assert await service.set_notify(person, result.request_id, True) + return result.request_id + + +async def set_status(engine: AsyncEngine, request_id: int, status: RequestStatus) -> None: + result = await PostgresFeatureRequestTriage(engine).triage( + request_id, TriageChange(status=status), actor="ana", now=NOW + ) + assert result.after is not None + + +def sweep(engine: AsyncEngine, messenger: Messenger) -> StatusNewsRunner: + return StatusNewsRunner(PostgresStatusNewsStore(engine), messenger, clock=lambda: NOW) + + +async def person_id(engine: AsyncEngine, person: PersonRef) -> int: + async with engine.connect() as conn: + found = await conn.scalar( + text( + "SELECT person_id FROM person_platform_id " + "WHERE platform = :p AND platform_user_id = :u" + ), + {"p": person.platform, "u": person.platform_user_id}, + ) + return int(found) + + +# --- the console's list and change ---------------------------------------------- + + +async def test_the_list_is_newest_first_with_names_and_same_text_counts( + clean: AsyncEngine, +) -> None: + first = await suggest(clean, LEO, "Dark mode, please", notify=False, name="Leo") + second = await suggest(clean, ANA, "dark mode please", notify=False, name="Ana") + await set_status(clean, first, RequestStatus.PLANNED) + + page = await PostgresFeatureRequestTriage(clean).triage_page(None, offset=0, limit=10) + + assert page.total == 2 + assert [e.id for e in page.entries] == [second, first] + assert [e.person_name for e in page.entries] == ["Ana", "Leo"] + assert [e.same_text_elsewhere for e in page.entries] == [1, 1] + planned = await PostgresFeatureRequestTriage(clean).triage_page( + RequestStatus.PLANNED, offset=0, limit=10 + ) + assert (planned.total, [e.id for e in planned.entries]) == (1, [first]) + + +async def test_a_change_returns_the_row_before_and_after(clean: AsyncEngine) -> None: + original = await suggest(clean, LEO, "dark mode", notify=False) + copy = await suggest(clean, ANA, "night mode", notify=False) + + result = await PostgresFeatureRequestTriage(clean).triage( + copy, + TriageChange( + status=RequestStatus.DUPLICATE, + set_note=True, + admin_note="same as dark mode", + set_duplicate=True, + duplicate_of=original, + ), + actor="oidc:ana-sub", + now=NOW + timedelta(hours=1), + ) + + assert result.before is not None and result.after is not None + assert result.before.status is RequestStatus.NEW + assert result.after.status is RequestStatus.DUPLICATE + assert result.after.duplicate_of == original + assert result.after.admin_note == "same as dark mode" + assert result.after.updated_by == "oidc:ana-sub" + assert result.after.updated_at == NOW + timedelta(hours=1) + + +async def test_an_unknown_row_or_duplicate_target_changes_nothing(clean: AsyncEngine) -> None: + mine = await suggest(clean, LEO, "dark mode", notify=False) + triage = PostgresFeatureRequestTriage(clean) + + missing = await triage.triage( + mine + 100, TriageChange(status=RequestStatus.DONE), actor="ana", now=NOW + ) + unknown = await triage.triage( + mine, + TriageChange(status=RequestStatus.DUPLICATE, set_duplicate=True, duplicate_of=mine + 100), + actor="ana", + now=NOW, + ) + + assert missing.refusal is TriageRefusal.NOT_FOUND + assert unknown.refusal is TriageRefusal.UNKNOWN_DUPLICATE + [entry] = (await triage.triage_page(None, offset=0, limit=10)).entries + assert entry.status is RequestStatus.NEW + + +# --- status news ------------------------------------------------------------------ + + +async def test_one_message_per_status_change_to_an_author_who_asked(clean: AsyncEngine) -> None: + mine = await suggest(clean, LEO, "dark mode", notify=True) + messenger = Messenger() + + assert await sweep(clean, messenger).run_due() == 0 # nothing has changed yet + await set_status(clean, mine, RequestStatus.PLANNED) + assert await sweep(clean, messenger).run_due() == 1 + assert await sweep(clean, messenger).run_due() == 0 + await set_status(clean, mine, RequestStatus.DONE) + assert await sweep(clean, messenger).run_due() == 1 + + assert [(p, i) for p, i, _ in messenger.sent] == [(LEO, mine), (LEO, mine)] + assert "**planned**" in messenger.sent[0][2] + assert "**done**" in messenger.sent[1][2] + + +async def test_no_message_without_the_yes(clean: AsyncEngine) -> None: + theirs = await suggest(clean, ANA, "light mode", notify=False) + await set_status(clean, theirs, RequestStatus.PLANNED) + messenger = Messenger() + + assert await sweep(clean, messenger).run_due() == 0 + assert messenger.sent == [] + + +async def test_a_change_undone_before_the_sweep_sends_nothing(clean: AsyncEngine) -> None: + mine = await suggest(clean, LEO, "dark mode", notify=True) + await set_status(clean, mine, RequestStatus.PLANNED) + await set_status(clean, mine, RequestStatus.NEW) + messenger = Messenger() + + assert await sweep(clean, messenger).run_due() == 0 + + +async def test_undeliverable_people_are_skipped_until_they_reopen(clean: AsyncEngine) -> None: + mine = await suggest(clean, LEO, "dark mode", notify=True) + await set_status(clean, mine, RequestStatus.PLANNED) + closed = Messenger(DeliveryResult.CLOSED) + + assert await sweep(clean, closed).run_due() == 0 + # Recorded as undeliverable, and the claim put back: the next sweep does + # not even try. + async with clean.connect() as conn: + marked = await conn.scalar( + text("SELECT undeliverable_at FROM notification_preference WHERE person_id = :i"), + {"i": await person_id(clean, LEO)}, + ) + assert marked == NOW + opened = Messenger() + assert await sweep(clean, opened).run_due() == 0 + assert opened.sent == [] + + # `/notifications on` clears the mark; the change is then told once. + await PostgresNotificationQueue(clean).set_enabled(LEO, True) + assert await sweep(clean, opened).run_due() == 1 + + +async def test_a_transient_failure_is_retried(clean: AsyncEngine) -> None: + mine = await suggest(clean, LEO, "dark mode", notify=True) + await set_status(clean, mine, RequestStatus.PLANNED) + + assert await sweep(clean, Messenger(DeliveryResult.FAILED)).run_due() == 0 + assert await sweep(clean, Messenger()).run_due() == 1 + + +async def test_notifications_off_means_no_message(clean: AsyncEngine) -> None: + mine = await suggest(clean, LEO, "dark mode", notify=True) + await PostgresNotificationQueue(clean).set_enabled(LEO, False) + await set_status(clean, mine, RequestStatus.PLANNED) + + assert await sweep(clean, Messenger()).run_due() == 0 + + +async def test_nothing_is_sent_after_an_opt_out(clean: AsyncEngine) -> None: + mine = await suggest(clean, LEO, "dark mode", notify=True) + await set_status(clean, mine, RequestStatus.PLANNED) + + await OptOutService(PostgresRetentionStore(clean)).opt_out(LEO, "test") + + assert await sweep(clean, Messenger()).run_due() == 0 + + +async def test_nothing_is_sent_after_erasure(clean: AsyncEngine) -> None: + mine = await suggest(clean, LEO, "dark mode", notify=True) + await set_status(clean, mine, RequestStatus.PLANNED) + + async with clean.begin() as conn: + await conn.execute( + text("SELECT purge_person_derived(:i)"), {"i": await person_id(clean, LEO)} + ) + + assert await sweep(clean, Messenger()).run_due() == 0 + + +async def test_no_message_after_yes_then_no(clean: AsyncEngine) -> None: + # Answering No after Yes leaves `notified_status` set; only the opt-in + # guard keeps the change from being announced. + mine = await suggest(clean, LEO, "dark mode", notify=True) + service = FeatureRequestService(PostgresFeatureRequestStore(clean), clock=lambda: NOW) + assert await service.set_notify(LEO, mine, False) + await set_status(clean, mine, RequestStatus.PLANNED) + messenger = Messenger() + + assert await sweep(clean, messenger).run_due() == 0 + assert messenger.sent == [] + + +async def test_an_opted_out_persons_leftover_rows_neither_send_nor_block( + clean: AsyncEngine, +) -> None: + # The purge normally deletes the rows. If it ever missed them, the claim + # must skip them itself: the row guard would drop the UPDATE, but rows + # picked and then dropped would fill every batch and starve everyone else. + theirs = await suggest(clean, LEO, "dark mode", notify=True) + mine = await suggest(clean, ANA, "light mode", notify=True) + await set_status(clean, theirs, RequestStatus.PLANNED) + await set_status(clean, mine, RequestStatus.PLANNED) + async with clean.begin() as conn: + await conn.execute(text("ALTER TABLE person_opt_out DISABLE TRIGGER USER")) + await conn.execute( + text("INSERT INTO person_opt_out (person_id) VALUES (:i)"), + {"i": await person_id(clean, LEO)}, + ) + await conn.execute(text("ALTER TABLE person_opt_out ENABLE TRIGGER USER")) + messenger = Messenger() + + runner = StatusNewsRunner( + PostgresStatusNewsStore(clean), messenger, clock=lambda: NOW, batch=1 + ) + assert await runner.run_due() == 1 + assert [(p, i) for p, i, _ in messenger.sent] == [(ANA, mine)] + + +async def test_the_message_goes_to_the_account_the_suggestion_came_from( + clean: AsyncEngine, +) -> None: + mine = await suggest(clean, LEO, "dark mode", notify=True) + # The same person on another platform, with a lower account id. + async with clean.begin() as conn: + await conn.execute( + text( + "INSERT INTO person_platform_id (platform, platform_user_id, person_id) " + "VALUES ('another', 1, :i)" + ), + {"i": await person_id(clean, LEO)}, + ) + await set_status(clean, mine, RequestStatus.PLANNED) + messenger = Messenger() + + assert await sweep(clean, messenger).run_due() == 1 + assert [p for p, _, _ in messenger.sent] == [LEO] + + +async def test_a_late_put_back_does_not_undo_a_newer_claim(clean: AsyncEngine) -> None: + mine = await suggest(clean, LEO, "dark mode", notify=True) + await set_status(clean, mine, RequestStatus.PLANNED) + store = PostgresStatusNewsStore(clean) + [stale] = await store.claim_status_news(10) + # While the first message is in flight the status moves on and another + # sweep claims and sends the newer change. + await set_status(clean, mine, RequestStatus.DONE) + [newer] = await store.claim_status_news(10) + assert newer.status is RequestStatus.DONE + + await store.release(stale) + + assert await store.claim_status_news(10) == [] + + +async def test_a_raised_delivery_loses_no_claim(clean: AsyncEngine) -> None: + # Regression: an exception from deliver() abandoned the batch with every + # claim advanced, so those authors were never told. + first = await suggest(clean, LEO, "dark mode", notify=True) + second = await suggest(clean, ANA, "light mode", notify=True) + await set_status(clean, first, RequestStatus.PLANNED) + await set_status(clean, second, RequestStatus.PLANNED) + + class Raising(Messenger): + async def deliver(self, person: PersonRef, task_id: int, text: str) -> DeliveryResult: + raise OSError("connection reset") + + assert await sweep(clean, Raising()).run_due() == 0 + working = Messenger() + assert await sweep(clean, working).run_due() == 2 + assert sorted(i for _, i, _ in working.sent) == sorted([first, second]) + + +async def test_the_message_is_in_the_suggestions_language(clean: AsyncEngine) -> None: + mine = await suggest(clean, LEO, "tenho uma ideia: modo escuro para o painel", notify=True) + await set_status(clean, mine, RequestStatus.PLANNED) + messenger = Messenger() + + await sweep(clean, messenger).run_due() + + [(_, _, message)] = messenger.sent + assert message.startswith(f"Sua sugestão **#{mine}** agora está: **planejada**.") diff --git a/tests/unit/test_admin_api.py b/tests/unit/test_admin_api.py index 1a2bed4..7fdcd26 100644 --- a/tests/unit/test_admin_api.py +++ b/tests/unit/test_admin_api.py @@ -14,7 +14,7 @@ from __future__ import annotations from collections.abc import Sequence -from dataclasses import dataclass, field +from dataclasses import dataclass, field, replace from datetime import UTC, datetime import pytest @@ -42,6 +42,15 @@ from chatmemory.domain.identity import PersonRef from chatmemory.entrypoints.admin import environment_baseline from chatmemory.ports.configuration import StoredSetting +from chatmemory.ports.feature_requests import ( + RequestStatus, + SourceKind, + TriageChange, + TriageEntry, + TriagePage, + TriageRefusal, + TriageResult, +) from tests.unit.usage_fakes import FakeUsageDirectory, FakeUsageSource ANA = Operator("ana") @@ -202,6 +211,63 @@ async def active_tokens(self) -> Sequence[TokenRecord]: return await self.store.active_tokens() +class FakeFeatureRequests: + """The console's triage port over a dict of rows.""" + + def __init__(self, rows: Sequence[TriageEntry] = ()) -> None: + self.rows = {row.id: row for row in rows} + self.changes: list[tuple[int, TriageChange, str]] = [] + + async def triage_page( + self, status: RequestStatus | None, *, offset: int, limit: int + ) -> TriagePage: + matching = [ + row + for row in sorted(self.rows.values(), key=lambda r: r.id, reverse=True) + if status is None or row.status is status + ] + return TriagePage(entries=matching[offset : offset + limit], total=len(matching)) + + async def triage( + self, request_id: int, change: TriageChange, *, actor: str, now: datetime + ) -> TriageResult: + before = self.rows.get(request_id) + if before is None: + return TriageResult(refusal=TriageRefusal.NOT_FOUND) + if change.set_duplicate and change.duplicate_of not in (None, *self.rows): + return TriageResult(before=before, refusal=TriageRefusal.UNKNOWN_DUPLICATE) + after = replace( + before, + status=change.status or before.status, + admin_note=change.admin_note if change.set_note else before.admin_note, + duplicate_of=change.duplicate_of if change.set_duplicate else before.duplicate_of, + updated_at=now, + updated_by=actor, + ) + self.rows[request_id] = after + self.changes.append((request_id, change, actor)) + return TriageResult(before=before, after=after) + + +def suggestion( + request_id: int, status: RequestStatus = RequestStatus.NEW, text: str = "dark mode" +) -> TriageEntry: + return TriageEntry( + id=request_id, + text=text, + language="en", + status=status, + admin_note=None, + duplicate_of=None, + source_kind=SourceKind.COMMAND, + person_name="Sam", + same_text_elsewhere=0, + created_at=AT, + updated_at=AT, + updated_by=None, + ) + + # --- the harness ------------------------------------------------------- @@ -219,6 +285,7 @@ class Console: optouts: FakeOptOutRegistry channels: FakeChannelDirectory credential: str + feature_requests: FakeFeatureRequests def auth(self, token: str | None = None) -> dict[str, str]: return {"Authorization": f"Bearer {token or self.credential}"} @@ -236,6 +303,7 @@ async def build_console( status: StatusSnapshot | None = None, oidc_configured: bool = False, sign_in: SignIn | None = None, + suggestions: Sequence[TriageEntry] = (), usage: UsageService | None = None, ) -> Console: tokens = InMemoryOperatorTokens() @@ -248,6 +316,7 @@ async def build_console( registry = FakeOptOutRegistry() mcp_tokens = InMemoryTokenStore() directory = FakeChannelDirectory(channels) + feature_requests = FakeFeatureRequests(suggestions) services = AdminServices( configuration=configuration, @@ -259,6 +328,7 @@ async def build_console( optouts=OptOutService(registry, FakeDocumentPurge(), FakePersonTraces()), mcp_tokens=ReviewAndRevokeOnly(mcp_tokens), probe=probe, + feature_requests=feature_requests, usage=usage or UsageService(FakeUsageSource(), FakeUsageDirectory(), retention_days=90), ) app = build_app(services, tokens, oidc_configured=oidc_configured, sign_in=sign_in) @@ -275,6 +345,7 @@ async def build_console( optouts=registry, channels=directory, credential=issued.token, + feature_requests=feature_requests, ) diff --git a/tests/unit/test_admin_api_wiring.py b/tests/unit/test_admin_api_wiring.py index 97b3c59..64ae31a 100644 --- a/tests/unit/test_admin_api_wiring.py +++ b/tests/unit/test_admin_api_wiring.py @@ -32,6 +32,7 @@ PostgresSessionStore, ) from chatmemory.adapters.store.config_postgres import PostgresConfigurationStore +from chatmemory.adapters.store.feature_requests_postgres import PostgresFeatureRequestTriage from chatmemory.adapters.store.trace_postgres import PostgresTraceIndex from chatmemory.admin.auth import Operator from chatmemory.admin.handlers.queries import ( @@ -83,6 +84,8 @@ # too -- a screen that offered to issue one would have no server behind it. ("DELETE", "/api/tokens/{id}"), ("GET", "/api/audit"), + ("GET", "/api/feature-requests"), + ("PATCH", "/api/feature-requests/{id}"), ] @@ -103,6 +106,7 @@ def test_the_service_is_built_from_the_postgres_adapters() -> None: assert isinstance(process.services.changes, PostgresChangeRecord) assert isinstance(process.services.mcp_tokens, PostgresTokenStore) assert isinstance(process.services.editor.store, PostgresConfigurationStore) + assert isinstance(process.services.feature_requests, PostgresFeatureRequestTriage) def test_the_opt_out_service_covers_documents() -> None: diff --git a/tests/unit/test_admin_feature_requests.py b/tests/unit/test_admin_feature_requests.py new file mode 100644 index 0000000..0460e45 --- /dev/null +++ b/tests/unit/test_admin_feature_requests.py @@ -0,0 +1,187 @@ +"""The console's feature-request screen: operators read, admins triage, all recorded.""" + +from __future__ import annotations + +from chatmemory.admin.audit import ChangeKind +from chatmemory.ports.feature_requests import RequestStatus +from tests.unit.test_admin_api import build_console, suggestion + +REQUIRES_ADMIN = {"error": "requires admin"} + + +async def test_operators_read_the_list_newest_first() -> None: + console = await build_console( + oidc_configured=True, suggestions=[suggestion(1), suggestion(2, text="voice notes")] + ) + + response = console.client.get("/api/feature-requests", headers=console.auth()) + + assert response.status_code == 200 + body = response.json() + assert [item["id"] for item in body["items"]] == [2, 1] + assert body["total"] == 2 + assert body["items"][0] == { + "id": 2, + "text": "voice notes", + "language": "en", + "status": "new", + "admin_note": None, + "duplicate_of": None, + "source_kind": "command", + "person": "Sam", + "same_text_elsewhere": 0, + "created_at": "2026-01-01T00:00:00+00:00", + "updated_at": "2026-01-01T00:00:00+00:00", + "updated_by": None, + } + + +async def test_the_list_filters_by_status() -> None: + console = await build_console( + suggestions=[suggestion(1), suggestion(2, RequestStatus.PLANNED)] + ) + + response = console.client.get( + "/api/feature-requests?status=planned", headers=console.auth() + ) + + assert [item["id"] for item in response.json()["items"]] == [2] + + +async def test_an_unknown_status_filter_is_refused() -> None: + console = await build_console() + + response = console.client.get("/api/feature-requests?status=shipped", headers=console.auth()) + + assert response.status_code == 400 + assert "status must be one of" in response.json()["error"] + + +async def test_an_operator_cannot_triage() -> None: + console = await build_console(oidc_configured=True, suggestions=[suggestion(1)]) + + response = console.client.patch( + "/api/feature-requests/1", json={"status": "planned"}, headers=console.auth() + ) + + assert (response.status_code, response.json()) == (403, REQUIRES_ADMIN) + assert console.feature_requests.changes == [] + assert console.feature_requests.rows[1].status is RequestStatus.NEW + assert console.changes._entries == [] # noqa: SLF001 + + +async def test_an_admin_sets_the_status_and_the_change_is_recorded() -> None: + console = await build_console(suggestions=[suggestion(7)]) + + response = console.client.patch( + "/api/feature-requests/7", json={"status": "planned"}, headers=console.auth() + ) + + assert response.status_code == 200 + assert response.json()["request"]["status"] == "planned" + assert response.json()["request"]["updated_by"] == "ana" + [entry] = await console.changes.recent() + assert (entry.operator, entry.setting, entry.kind) == ( + "ana", + "feature_request.7.status", + ChangeKind.APPLIED, + ) + assert (entry.before, entry.after) == ("new", "planned") + + +async def test_the_note_is_recorded_as_set_never_as_its_text() -> None: + console = await build_console(suggestions=[suggestion(7)]) + + console.client.patch( + "/api/feature-requests/7", + json={"admin_note": "talk to Bea about the API budget"}, + headers=console.auth(), + ) + + [entry] = await console.changes.recent() + assert entry.setting == "feature_request.7.admin_note" + assert (entry.before, entry.after) == ("empty", "set") + assert console.feature_requests.rows[7].admin_note == "talk to Bea about the API budget" + + +async def test_marking_a_duplicate_records_both_fields() -> None: + console = await build_console(suggestions=[suggestion(3), suggestion(7)]) + + response = console.client.patch( + "/api/feature-requests/7", + json={"status": "duplicate", "duplicate_of": 3}, + headers=console.auth(), + ) + + assert response.status_code == 200 + settings = {e.setting: (e.before, e.after) for e in await console.changes.recent()} + assert settings == { + "feature_request.7.status": ("new", "duplicate"), + "feature_request.7.duplicate_of": (None, "#3"), + } + + +async def test_an_unchanged_value_records_nothing() -> None: + console = await build_console(suggestions=[suggestion(7)]) + + response = console.client.patch( + "/api/feature-requests/7", json={"status": "new"}, headers=console.auth() + ) + + assert response.status_code == 200 + assert await console.changes.recent() == () + + +async def test_the_text_cannot_be_changed() -> None: + console = await build_console(suggestions=[suggestion(7)]) + + response = console.client.patch( + "/api/feature-requests/7", json={"text": "something else"}, headers=console.auth() + ) + + assert response.status_code == 400 + assert console.feature_requests.changes == [] + + +async def test_bad_values_are_refused_before_anything_changes() -> None: + console = await build_console(suggestions=[suggestion(7)]) + bad = [ + {}, + {"status": "shipped"}, + {"admin_note": 12}, + {"admin_note": "x" * 2001}, + {"duplicate_of": 7}, + {"duplicate_of": True}, + {"duplicate_of": "seven"}, + ] + + for body in bad: + response = console.client.patch( + "/api/feature-requests/7", json=body, headers=console.auth() + ) + assert response.status_code == 400, body + + assert console.feature_requests.changes == [] + assert await console.changes.recent() == () + + +async def test_an_unknown_duplicate_target_is_refused() -> None: + console = await build_console(suggestions=[suggestion(7)]) + + response = console.client.patch( + "/api/feature-requests/7", json={"duplicate_of": 99}, headers=console.auth() + ) + + assert response.status_code == 400 + assert await console.changes.recent() == () + + +async def test_an_unknown_suggestion_is_404() -> None: + console = await build_console() + + response = console.client.patch( + "/api/feature-requests/5", json={"status": "done"}, headers=console.auth() + ) + + assert response.status_code == 404 + assert await console.changes.recent() == () diff --git a/tests/unit/test_admin_roles.py b/tests/unit/test_admin_roles.py index eff34c7..12c6ca2 100644 --- a/tests/unit/test_admin_roles.py +++ b/tests/unit/test_admin_roles.py @@ -224,6 +224,7 @@ async def test_with_an_issuer_a_token_is_refused_on_every_admin_route() -> None: "/api/channels", "/api/optouts", "/api/tokens", + "/api/feature-requests", "/api/usage/summary", } diff --git a/tests/unit/test_feature_request_news.py b/tests/unit/test_feature_request_news.py new file mode 100644 index 0000000..35d57dc --- /dev/null +++ b/tests/unit/test_feature_request_news.py @@ -0,0 +1,193 @@ +"""The suggestion status sweep: one message per change, never an unsent claim kept.""" + +from __future__ import annotations + +import ast +import asyncio +from collections.abc import Sequence +from dataclasses import dataclass, field +from datetime import UTC, datetime + +import pytest + +from chatmemory.app.feature_request_news import StatusNewsRunner, status_message +from chatmemory.domain.identity import PersonRef +from chatmemory.entrypoints.bot import suggestion_news_loop +from chatmemory.health import HealthState +from chatmemory.ports.feature_requests import RequestStatus, StatusNews +from chatmemory.ports.notifications import DeliveryResult +from tests.unit.test_assembly_seam import BOT, _calls, _function + +NOW = datetime(2026, 9, 26, 12, tzinfo=UTC) +SAM = PersonRef("discord", 42) + + +def news(request_id: int = 12, language: str | None = "en", text: str = "dark mode") -> StatusNews: + return StatusNews( + request_id=request_id, + person=SAM, + status=RequestStatus.PLANNED, + previous="new", + language=language, + text=text, + ) + + +@dataclass +class FakeStore: + """Hands out each queued item once, as the claim does.""" + + queued: list[StatusNews] = field(default_factory=list) + released: list[StatusNews] = field(default_factory=list) + undeliverable: list[PersonRef] = field(default_factory=list) + + async def claim_status_news(self, limit: int) -> Sequence[StatusNews]: + claimed, self.queued = self.queued[:limit], self.queued[limit:] + return claimed + + async def release(self, item: StatusNews) -> None: + self.released.append(item) + self.queued.append(item) + + async def record_undeliverable(self, person: PersonRef, now: datetime) -> None: + self.undeliverable.append(person) + + +@dataclass +class FakeMessenger: + result: DeliveryResult = DeliveryResult.SENT + sent: list[tuple[PersonRef, int, str]] = field(default_factory=list) + + async def deliver(self, person: PersonRef, task_id: int, text: str) -> DeliveryResult: + if self.result is DeliveryResult.SENT: + self.sent.append((person, task_id, text)) + return self.result + + +def runner(store: FakeStore, messenger: FakeMessenger) -> StatusNewsRunner: + return StatusNewsRunner(store, messenger, clock=lambda: NOW) + + +async def test_a_change_is_sent_once() -> None: + store, messenger = FakeStore([news()]), FakeMessenger() + + assert await runner(store, messenger).run_due() == 1 + assert await runner(store, messenger).run_due() == 0 + + assert [(p, i) for p, i, _ in messenger.sent] == [(SAM, 12)] + assert store.released == [] + + +async def test_a_closed_direct_message_is_put_back_and_recorded() -> None: + store, messenger = FakeStore([news()]), FakeMessenger(DeliveryResult.CLOSED) + + assert await runner(store, messenger).run_due() == 0 + + assert store.released == [news()] + assert store.undeliverable == [SAM] + + +async def test_a_transient_failure_is_put_back_for_the_next_sweep() -> None: + store, messenger = FakeStore([news()]), FakeMessenger(DeliveryResult.FAILED) + + await runner(store, messenger).run_due() + + assert store.released == [news()] + assert store.undeliverable == [] + messenger.result = DeliveryResult.SENT + assert await runner(store, messenger).run_due() == 1 + + +@dataclass +class RaisingMessenger(FakeMessenger): + """Raises on the first delivery, as a dropped socket or a timeout would.""" + + raised: int = 0 + + async def deliver(self, person: PersonRef, task_id: int, text: str) -> DeliveryResult: + if self.raised == 0: + self.raised += 1 + raise OSError("connection reset") + return await super().deliver(person, task_id, text) + + +async def test_a_raised_delivery_is_put_back_and_the_batch_goes_on() -> None: + # Regression: an exception from deliver() left the loop, and every claim + # after it stayed advanced, so those messages were never sent. + store, messenger = FakeStore([news(1), news(2)]), RaisingMessenger() + + assert await runner(store, messenger).run_due() == 1 + assert [i for _, i, _ in messenger.sent] == [2] + assert store.released == [news(1)] + + assert await runner(store, messenger).run_due() == 1 + assert sorted(i for _, i, _ in messenger.sent) == [1, 2] + + +class FailingReleaseStore(FakeStore): + async def release(self, item: StatusNews) -> None: + self.released.append(item) + raise OSError("database went away") + + +async def test_a_failed_put_back_does_not_abandon_the_rest() -> None: + store = FailingReleaseStore([news(1), news(2)]) + + assert await runner(store, FakeMessenger(DeliveryResult.FAILED)).run_due() == 0 + + assert [n.request_id for n in store.released] == [1, 2] + + +def test_the_message_is_in_the_suggestions_language() -> None: + assert status_message(news(language="pt")) == ( + "Sua sugestão **#12** agora está: **planejada**.\n> dark mode\n" + "`/suggestions` mostra todas as suas." + ) + assert status_message(news(language=None)).startswith( + "Your suggestion **#12** is now **planned**." + ) + + +def test_a_long_suggestion_is_clipped_to_one_line() -> None: + message = status_message(news(text="line one\nline two " + "x" * 300)) + + quoted = message.splitlines()[1] + assert quoted.startswith("> line one line two") + assert quoted.endswith("…") + assert len(quoted) == 2 + 200 + + +# --- wiring: built beside /suggest, started beside the gateway ---------------------- + + +def test_build_bot_builds_the_sweep_with_an_unheaded_messenger() -> None: + [call] = _calls(_function(BOT, "build_bot"), "build_feature_request_news") + messenger = call.args[1] + assert ast.unparse(messenger) == "DiscordTaskMessenger(client.fetch_user, prefix='')" + + +def test_main_starts_the_sweep_only_when_it_was_built() -> None: + main = _function(BOT, "main") + [call] = _calls(main, "suggestion_news_loop") + ready = next(k.value for k in call.keywords if k.arg == "ready") + assert getattr(ready, "id", None) == "gateway_ready" + [guard] = [ + node + for node in ast.walk(main) + if isinstance(node, ast.If) + and ast.unparse(node.test) == "graph.suggestion_news is not None" + ] + guarded = [c for statement in guard.body for c in _calls(statement, "suggestion_news_loop")] + assert guarded == [call] + + +async def test_the_sweep_waits_for_the_gateway() -> None: + store, messenger = FakeStore([news()]), FakeMessenger() + task = asyncio.ensure_future( + suggestion_news_loop(runner(store, messenger), HealthState(), ready=asyncio.Event()) + ) + await asyncio.sleep(0.01) + task.cancel() + with pytest.raises(asyncio.CancelledError): + await task + assert messenger.sent == [] diff --git a/tests/unit/test_sql_audit.py b/tests/unit/test_sql_audit.py index c5393c0..ab70d85 100644 --- a/tests/unit/test_sql_audit.py +++ b/tests/unit/test_sql_audit.py @@ -501,6 +501,37 @@ def discover() -> dict[str, TextClause]: "write; the requester's answer to 'tell you when its status changes?', " "keyed on their person id as well as the row id. Returns no row" ), + "feature_requests_sql.TRIAGE_PAGE": ( + "the admin console's list of every suggestion, with the author's display " + "name and a count of others with the same text. A suggestion is the " + "person's own words given to the team with a disclosure, never channel " + "content, so there is no channel to scope by" + ), + "feature_requests_sql.TRIAGE_COUNT": ( + "a count of suggestions for the console's pager, optionally one status. " + "No content" + ), + "feature_requests_sql.TRIAGE_ROW": ( + "one suggestion by id for an admin's triage, locked so the change is " + "recorded against the row it was applied to; the person's own words, " + "never channel content" + ), + "feature_requests_sql.TRIAGE_EXISTS": ( + "a flag: whether the suggestion an admin names as the original exists" + ), + "feature_requests_sql.TRIAGE_UPDATE": ( + "write; an admin's status, note and duplicate link on one suggestion, " + "keyed on its id. Returns no row" + ), + "feature_requests_sql.CLAIM_STATUS_NEWS": ( + "write and read; claims status changes to announce to authors who opted " + "in, returning each author's own suggestion and platform id so the bot " + "can message them their own words. Never channel content" + ), + "feature_requests_sql.RELEASE_STATUS_NEWS": ( + "write; puts back a claim whose message was not sent, keyed on the row " + "id and the status it claimed. Returns no row" + ), # --- privacy_sql: /privacy, the asker's own rows ------------------------ "privacy_sql.IS_OPTED_OUT": ( "the asker's own opt-out flag, keyed on their person id"