From e1735b51e4fa8fe17468f4cf70e7c61ebd4a7f83 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 22:54:49 +0000 Subject: [PATCH] D608: a manual credit grant or deduction is the Super Admin's, through the grant service (#1308) POST /perks/admin/credits wrote the ledger itself: any admin, no reason, no audit row, no notice, a raw index error on a reused source, and rows the grants list never showed. It now calls manualAdjust in creditGrants.ts: Super Admin only, a reason of at least 10 characters in the audit row, a note of at most 500 the member sees, idempotent on source_ref (409 already_recorded naming the row), no overdraw (400 would_overdraw with the balance), both tests inside the insert, one logAdminAction row and one inbox notice. Manual lines carry rule_key 'manual' and show in the Super Admin's grants list, marked manual. No migration. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016Q3DcWsMTSxPxJa7tMeDAs --- .../src/routes/admin_credit_rules.ts | 10 +- cloudflare-worker/src/routes/perks.ts | 36 ++-- .../src/services/creditGrants.ts | 89 ++++++++ .../test/manual_credit_adjust_d608.test.ts | 192 ++++++++++++++++++ documentation/architecture/decisions/D540.md | 3 + documentation/architecture/decisions/D608.md | 97 +++++++++ frontend/test/perks_live.test.mjs | 8 +- 7 files changed, 409 insertions(+), 26 deletions(-) create mode 100644 cloudflare-worker/test/manual_credit_adjust_d608.test.ts create mode 100644 documentation/architecture/decisions/D608.md diff --git a/cloudflare-worker/src/routes/admin_credit_rules.ts b/cloudflare-worker/src/routes/admin_credit_rules.ts index 3281141b86..73ad701881 100644 --- a/cloudflare-worker/src/routes/admin_credit_rules.ts +++ b/cloudflare-worker/src/routes/admin_credit_rules.ts @@ -4,7 +4,8 @@ * * GET / every reward rule and the monthly bounty cap * PUT /:key change one rule's credits, value or on/off - * GET /grants rule-based grants, newest first (?user_id=) + * GET /grants rule-based grants and manual adjustments (D608, + * `manual: true`), newest first (?user_id=) * POST /grants/:id/revoke take one back * POST /lab-backfill pay the current Lab cohort once for milestones * completed before credits launched (D541, @@ -20,7 +21,7 @@ import { Hono } from 'hono'; import type { Env } from '../types'; import { requireSuperAdmin } from '../auth'; import { logAdminAction } from '../services/adminAudit'; -import { CAP_RULE_KEY, isBountyRule, revokeGrant } from '../services/creditGrants'; +import { CAP_RULE_KEY, MANUAL_RULE_KEY, isBountyRule, revokeGrant } from '../services/creditGrants'; import { backfillLabCredits } from '../services/labMilestoneCredits'; import { mapError } from './_t13t14t15_helpers'; import { refuse } from '../util/refusal'; @@ -84,7 +85,7 @@ r.get('/grants', async (c) => { LEFT JOIN users u ON u.id = g.user_id LEFT JOIN perk_credit_ledger r ON r.user_id = g.user_id AND r.kind = 'admin_adjust' AND r.source_ref = 'revoke:' || g.id - WHERE g.kind = 'grant' AND g.rule_key IS NOT NULL AND g.user_id = ? + WHERE ((g.kind = 'grant' AND g.rule_key IS NOT NULL) OR (g.kind = 'admin_adjust' AND g.rule_key = 'manual' AND substr(g.source_ref, 1, 7) <> 'revoke:')) AND g.user_id = ? ORDER BY g.created_at DESC, g.id DESC LIMIT 200`, ).bind(userId) : c.env.DB.prepare( @@ -94,13 +95,14 @@ r.get('/grants', async (c) => { LEFT JOIN users u ON u.id = g.user_id LEFT JOIN perk_credit_ledger r ON r.user_id = g.user_id AND r.kind = 'admin_adjust' AND r.source_ref = 'revoke:' || g.id - WHERE g.kind = 'grant' AND g.rule_key IS NOT NULL + WHERE ((g.kind = 'grant' AND g.rule_key IS NOT NULL) OR (g.kind = 'admin_adjust' AND g.rule_key = 'manual' AND substr(g.source_ref, 1, 7) <> 'revoke:')) ORDER BY g.created_at DESC, g.id DESC LIMIT 200`, ) ).all(); return c.json({ grants: (rows.results || []).map((g: any) => ({ id: g.id, user_id: g.user_id, email: g.email ?? null, credits: Number(g.credits), rule_key: g.rule_key, + manual: g.rule_key === MANUAL_RULE_KEY, source_ref: g.source_ref, note: g.note, created_at: g.created_at, revoked: g.revoke_id !== null && g.revoke_id !== undefined, revoke_reason: g.revoke_reason ?? null, revoked_at: g.revoked_at ?? null, diff --git a/cloudflare-worker/src/routes/perks.ts b/cloudflare-worker/src/routes/perks.ts index 62f558b5ea..a2237dc722 100644 --- a/cloudflare-worker/src/routes/perks.ts +++ b/cloudflare-worker/src/routes/perks.ts @@ -22,7 +22,7 @@ * ADMIN * GET /admin/queue everything awaiting review * POST /admin/:uid/review approve / reject / pause - * POST /admin/credits grant credits to a user + * POST /admin/credits a Super Admin's manual grant or deduction (D608) * * THE THREE PRICE KINDS, and why the credit one is the only interesting one: * @@ -59,11 +59,11 @@ import { Hono } from 'hono'; import { activeCompanyFor } from '../middleware/activeCompany'; import type { Env } from '../types'; -import { requireAuth, requireAdmin, requireRole } from '../auth'; +import { requireAuth, requireAdmin, requireRole, requireSuperAdmin } from '../auth'; import { mapError, newUid, nowIso, todayIso } from './_t13t14t15_helpers'; import { userMeetsTier, type Tier } from '../middleware/requireTier'; import { refuse } from '../util/refusal'; -import { ruleReason } from '../services/creditGrants'; +import { manualAdjust, ruleReason } from '../services/creditGrants'; const r = new Hono<{ Bindings: Env }>(); @@ -776,27 +776,23 @@ r.get('/admin/queue', async (c) => { } catch (e) { return mapError(c, e); } }); +// D608 (#1308): a manual grant or deduction is the Super Admin's, with a typed +// reason, and goes through the grant service like every other grant: one +// ledger line, idempotent on its source, an audit row and an inbox notice. r.post('/admin/credits', async (c) => { try { - const admin = await requireAdmin(c); + const admin = await requireSuperAdmin(c); const b = await c.req.json().catch(() => ({} as any)); - const userId = intOrNull(b?.user_id); - const delta = intOrNull(b?.delta); - if (!userId || !delta) return c.json({ error: 'user_id and a non-zero delta are required' }, 400); - const target = await c.env.DB.prepare('SELECT id FROM users WHERE id = ?') - .bind(userId).first<{ id: number }>(); - if (!target) return c.json({ error: 'not_found' }, 404); - // A grant must not take a balance negative — the ledger would then owe - // credits nobody can spend. - if (delta < 0 && (await balanceOf(c.env, userId)) + delta < 0) { - return c.json({ error: 'that would take the balance below zero' }, 400); + const out = await manualAdjust(c.env, { + userId: b?.user_id, delta: b?.delta, reason: b?.reason, note: b?.note, sourceRef: b?.source_ref, + actor: { id: admin.id, email: admin.email }, + }); + if (out.status === 'refused') { + const status = out.code === 'user_not_found' ? 404 : out.code === 'already_recorded' ? 409 : 400; + const extra = out.ledger_id !== undefined ? { ledger_id: out.ledger_id } : out.balance !== undefined ? { balance: out.balance } : {}; + return refuse(c, status, { code: out.code, message: out.message, extra }); } - const ref = str(b?.source_ref, 120) || `admin:${admin.id}:${nowIso()}`; - await c.env.DB.prepare( - `INSERT INTO perk_credit_ledger (user_id, delta, kind, source_ref, note, created_at) - VALUES (?,?,?,?,?,?)`, - ).bind(userId, delta, delta > 0 ? 'grant' : 'admin_adjust', ref, str(b?.note, 500) || null, nowIso()).run(); - return c.json({ ok: true, balance: await balanceOf(c.env, userId) }); + return c.json({ ok: true, ...out }); } catch (e) { return mapError(c, e); } }); diff --git a/cloudflare-worker/src/services/creditGrants.ts b/cloudflare-worker/src/services/creditGrants.ts index e47743fb87..e72a7ddfe0 100644 --- a/cloudflare-worker/src/services/creditGrants.ts +++ b/cloudflare-worker/src/services/creditGrants.ts @@ -277,3 +277,92 @@ export async function revokeGrant(env: Env, args: { { grant_ledger_id: grant.id, ledger_id: ledgerId, credits }); return { status: 'revoked', ledger_id: ledgerId, credits, notified }; } + +/** D608: the marker `rule_key` a Super Admin's manual adjustment carries, so the grants list can show it. */ +export const MANUAL_RULE_KEY = 'manual'; +/** The size of one manual adjustment, either way; the same bound a rule's credits take (`admin_credit_rules.ts`). */ +export const MANUAL_DELTA_MAX = 100_000; +/** The typed reason's floor, the one every Super Admin credit act shares (`CREDIT_REASON_MIN`). */ +export const MANUAL_REASON_MIN = 10; + +export type ManualRefusalCode = + | 'invalid_user' | 'invalid_delta' | 'reason_too_short' | 'note_too_long' | 'invalid_source_ref' + | 'user_not_found' | 'already_recorded' | 'would_overdraw'; + +export type ManualResult = + | { status: 'recorded'; ledger_id: number; delta: number; source_ref: string; balance: number; notified: boolean } + | { status: 'refused'; code: ManualRefusalCode; message: string; ledger_id?: number; balance?: number }; + +const balanceSql = 'SELECT COALESCE(SUM(delta), 0) AS b FROM perk_credit_ledger WHERE user_id = ?'; + +/** + * D608 — a Super Admin's manual grant or deduction. The route checks the + * caller is the Super Admin; everything else is here: + * - `delta` a non-zero whole number, at most MANUAL_DELTA_MAX either way; + * - a typed `reason` of at least MANUAL_REASON_MIN characters, in the audit row; + * - a `note` the member sees, at most 500 characters, refused rather than cut; + * - IDEMPOTENT ON `source_ref`: a ref this user's ledger already holds, of any + * kind, is `already_recorded` naming that row, never a raw index error; + * - a deduction that would take the balance below zero is `would_overdraw`. + * The balance test and the duplicate test are INSIDE the insert, so two + * adjustments racing cannot both pass them. + * A grant is a `grant` row, a deduction an `admin_adjust` row; both carry + * `rule_key = 'manual'`. It is not a bounty, so it never counts toward the cap. + */ +export async function manualAdjust(env: Env, args: { + userId: unknown; delta: unknown; reason: unknown; note?: unknown; sourceRef?: unknown; + actor: { id: number; email: string }; +}): Promise { + const refused = (code: ManualRefusalCode, message: string, extra: { ledger_id?: number; balance?: number } = {}): ManualResult => + ({ status: 'refused', code, message, ...extra }); + const userId = Number(args.userId); + if (!Number.isInteger(userId) || userId <= 0) return refused('invalid_user', 'Name the account by its user id.'); + const delta = typeof args.delta === 'number' ? args.delta : NaN; + if (!Number.isInteger(delta) || delta === 0 || Math.abs(delta) > MANUAL_DELTA_MAX) { + return refused('invalid_delta', `The change must be a whole number of credits, not zero, at most ${MANUAL_DELTA_MAX.toLocaleString('en-US')} either way.`); + } + const reason = clean(args.reason, NOTE_MAX); + if (reason.length < MANUAL_REASON_MIN) { + return refused('reason_too_short', `Say why — at least ${MANUAL_REASON_MIN} characters. It is recorded beside the change.`); + } + const rawNote = String(args.note ?? '').trim(); + if (rawNote.length > NOTE_MAX) return refused('note_too_long', `The note the member sees is at most ${NOTE_MAX} characters.`); + const rawRef = String(args.sourceRef ?? '').trim(); + if (rawRef.length > SOURCE_MAX) return refused('invalid_source_ref', `A source reference is at most ${SOURCE_MAX} characters.`); + const sourceRef = rawRef || `admin:${args.actor.id}:${new Date().toISOString()}`; + + const user = await env.DB.prepare('SELECT id FROM users WHERE id = ?').bind(userId).first<{ id: number }>(); + if (!user) return refused('user_not_found', 'No such account.'); + + const kind = delta > 0 ? 'grant' : 'admin_adjust'; + const note = rawNote || (delta > 0 ? 'Credits added by the Axal team' : 'Credits deducted by the Axal team'); + const ins = await env.DB.prepare( + `INSERT OR IGNORE INTO perk_credit_ledger (user_id, delta, kind, source_ref, note, created_at, rule_key) + SELECT ?, ?, ?, ?, ?, ?, ? + WHERE NOT EXISTS (SELECT 1 FROM perk_credit_ledger d WHERE d.user_id = ? AND d.source_ref = ?) + AND (SELECT COALESCE(SUM(b.delta), 0) FROM perk_credit_ledger b WHERE b.user_id = ?) + ? >= 0`, + ).bind(userId, delta, kind, sourceRef, note, new Date().toISOString(), MANUAL_RULE_KEY, + userId, sourceRef, userId, delta).run(); + + if (Number(ins?.meta?.changes ?? 0) !== 1) { + const prior = await env.DB.prepare( + 'SELECT id FROM perk_credit_ledger WHERE user_id = ? AND source_ref = ? ORDER BY id LIMIT 1', + ).bind(userId, sourceRef).first<{ id: number }>(); + if (prior) { + return refused('already_recorded', `This account's ledger already holds a line with the source ${sourceRef}, so nothing was recorded.`, { ledger_id: Number(prior.id) }); + } + const bal = Number((await env.DB.prepare(balanceSql).bind(userId).first<{ b: number }>())?.b) || 0; + return refused('would_overdraw', `The account holds ${plural(bal)}; deducting ${plural(-delta)} would take it below zero, so nothing was recorded.`, { balance: bal }); + } + + const ledgerId = Number(ins.meta.last_row_id); + await logAdminAction(env, args.actor.id, args.actor.email, 'credit_manual_adjust', { + target_user_id: userId, delta, ledger_id: ledgerId, source_ref: sourceRef, reason, note, + }); + const notified = await tell(env, userId, 'credit_manual_adjust', + delta > 0 ? `${plural(delta)} added to your account` : `${plural(-delta)} deducted from your account`, + `${note}. Credits have no cash value and cannot be transferred.`, + { ledger_id: ledgerId, delta }); + const balance = Number((await env.DB.prepare(balanceSql).bind(userId).first<{ b: number }>())?.b) || 0; + return { status: 'recorded', ledger_id: ledgerId, delta, source_ref: sourceRef, balance, notified }; +} diff --git a/cloudflare-worker/test/manual_credit_adjust_d608.test.ts b/cloudflare-worker/test/manual_credit_adjust_d608.test.ts new file mode 100644 index 0000000000..741736c03e --- /dev/null +++ b/cloudflare-worker/test/manual_credit_adjust_d608.test.ts @@ -0,0 +1,192 @@ +/** + * D608 (#1308) — a manual grant or deduction goes through the grant service: + * the Super Admin only, with a typed reason, one ledger line, one audit row, + * one inbox notice, idempotent on its source, and never below zero. The + * Super Admin's grants list shows it, marked manual. + * + * The schema is the Perks harness's (migrations 186, 198, 228, 322 and 381, + * read off disk). `POST /perks/admin/credits` and `GET /admin/credit-rules/grants` + * run through their real routers with real JWTs. + * + * node --experimental-strip-types --no-warnings --import ./cloudflare-worker/test/_ts-loader.mjs \ + * --test cloudflare-worker/test/manual_credit_adjust_d608.test.ts + */ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { SignJWT } from 'jose'; +import { JWT_SECRET, freshDb, makeD1, FOUNDER, ADMIN, balance } from './_perks_harness.ts'; +import { grantCredits, revokeGrant, manualAdjust } from '../src/services/creditGrants.ts'; +import creditRules from '../src/routes/admin_credit_rules.ts'; +import perks from '../src/routes/perks.ts'; + +const SUPER = 30; +const REASON = 'Goodwill for the outage on 3 October'; + +function setup() { + const db = freshDb(); + db.exec(`CREATE TABLE super_admins (user_id INTEGER PRIMARY KEY, granted_by_user_id INTEGER, granted_at TEXT); + CREATE TABLE push_subscriptions (id INTEGER PRIMARY KEY, user_id INTEGER, endpoint TEXT, p256dh TEXT, auth TEXT);`); + db.prepare(`INSERT INTO users (id, role, email) VALUES (?, 'admin', 'super@example.test')`).run(SUPER); + db.prepare('INSERT INTO super_admins (user_id) VALUES (?)').run(SUPER); + const DB: any = makeD1(db, { beforeBatch: null }); + return { db, env: { JWT_SECRET, ENVIRONMENT: 'development', DB } as any }; +} +async function tok(id: number, role: string) { + return new SignJWT({ user_id: id, role }).setProtectedHeader({ alg: 'HS256' }).setIssuedAt().setExpirationTime('1h') + .sign(new TextEncoder().encode(JWT_SECRET)); +} +const ROLES: Record = { [SUPER]: 'admin', [ADMIN]: 'admin', [FOUNDER]: 'founder' }; +async function call(router: any, env: any, method: string, path: string, who: number, body?: unknown) { + const headers: Record = { Authorization: `Bearer ${await tok(who, ROLES[who] || 'founder')}` }; + const init: RequestInit = { method, headers }; + if (body !== undefined) { headers['Content-Type'] = 'application/json'; init.body = JSON.stringify(body); } + const res = await router.request(path, init, env); + return { status: res.status, body: (await res.json().catch(() => null)) as any }; +} +const adjust = (env: any, who: number, body: unknown) => call(perks, env, 'POST', '/admin/credits', who, body); +const ledger = (db: any, userId: number) => + db.prepare('SELECT id, delta, kind, source_ref, note, rule_key FROM perk_credit_ledger WHERE user_id = ? ORDER BY id').all(userId).map((x: any) => ({ ...x })); +const inbox = (db: any, userId: number) => + db.prepare('SELECT type, title, body FROM notifications_inbox WHERE user_id = ? ORDER BY id').all(userId).map((x: any) => ({ ...x })); +const audit = (db: any) => + db.prepare("SELECT details FROM activity_logs WHERE action = 'credit_manual_adjust' ORDER BY id").all().map((x: any) => JSON.parse(x.details)); +const rowCounts = (db: any) => [ + (db.prepare('SELECT COUNT(*) AS n FROM perk_credit_ledger').get() as any).n, + (db.prepare("SELECT COUNT(*) AS n FROM activity_logs WHERE action = 'credit_manual_adjust'").get() as any).n, + inboxCount(db), +]; +// The inbox table is created by the first notice, so before one it holds none. +function inboxCount(db: any) { + try { return (db.prepare('SELECT COUNT(*) AS n FROM notifications_inbox').get() as any).n; } catch { return 0; } +} + +test('only the Super Admin adjusts: a plain admin and a founder are refused, and nothing is written', async () => { + const { db, env } = setup(); + for (const who of [ADMIN, FOUNDER]) { + const r = await adjust(env, who, { user_id: FOUNDER, delta: 50, reason: REASON }); + assert.equal(r.status, 403, `user ${who}`); + } + assert.deepEqual(rowCounts(db), [0, 0, 0]); +}); + +test('a grant writes one ledger line, one audit row with the reason, and one inbox notice', async () => { + const { db, env } = setup(); + const r = await adjust(env, SUPER, { user_id: FOUNDER, delta: 50, reason: REASON, note: 'Thanks for your patience' }); + assert.equal(r.status, 200); + assert.equal(r.body.status, 'recorded'); + assert.equal(r.body.balance, 50); + assert.equal(r.body.notified, true); + assert.match(r.body.source_ref, new RegExp(`^admin:${SUPER}:\\d{4}-\\d{2}-\\d{2}T`), 'the default source names the admin and the time'); + assert.deepEqual(ledger(db, FOUNDER).map((l: any) => [l.id, l.delta, l.kind, l.note, l.rule_key]), + [[r.body.ledger_id, 50, 'grant', 'Thanks for your patience', 'manual']]); + const a = audit(db); + assert.equal(a.length, 1); + assert.deepEqual([a[0].target_user_id, a[0].delta, a[0].ledger_id, a[0].reason, a[0].source_ref], + [FOUNDER, 50, r.body.ledger_id, REASON, r.body.source_ref]); + const n = inbox(db, FOUNDER); + assert.deepEqual(n.map((x: any) => [x.type, x.title]), [['credit_manual_adjust', '50 credits added to your account']]); + assert.match(n[0].body, /^Thanks for your patience\. Credits have no cash value/); + assert.doesNotMatch(n[0].body, /outage/, 'the reason is the audit\'s, not the member\'s'); +}); + +test('a deduction writes an admin_adjust line, audits it and tells the member', async () => { + const { db, env } = setup(); + await adjust(env, SUPER, { user_id: FOUNDER, delta: 80, reason: REASON, source_ref: 'seed' }); + const r = await adjust(env, SUPER, { user_id: FOUNDER, delta: -30, reason: 'Duplicate goodwill grant, reversed' }); + assert.equal(r.status, 200); + assert.equal(r.body.balance, 50); + assert.equal(balance(db, FOUNDER), 50); + const last = ledger(db, FOUNDER).at(-1); + assert.deepEqual([last.delta, last.kind, last.rule_key, last.note], [-30, 'admin_adjust', 'manual', 'Credits deducted by the Axal team']); + assert.equal(audit(db).at(-1).delta, -30); + assert.equal(inbox(db, FOUNDER).at(-1).title, '30 credits deducted from your account'); + assert.deepEqual(rowCounts(db), [2, 2, 2]); +}); + +test('the reason is required: missing or under ten characters is refused, and nothing is written', async () => { + const { db, env } = setup(); + for (const reason of [undefined, '', ' too short ', '123456789']) { + const r = await adjust(env, SUPER, { user_id: FOUNDER, delta: 10, reason }); + assert.equal(r.status, 400, String(reason)); + assert.equal(r.body.error, 'reason_too_short'); + } + assert.equal((await adjust(env, SUPER, { user_id: FOUNDER, delta: 10, reason: '1234567890' })).status, 200, 'ten characters is enough'); + assert.deepEqual(rowCounts(db), [1, 1, 1]); +}); + +test('the change is a non-zero whole number within the bound, and the note at most 500 characters', async () => { + const { db, env } = setup(); + for (const delta of [0, 1.5, '10', 100_001, -100_001, null]) { + const r = await adjust(env, SUPER, { user_id: FOUNDER, delta, reason: REASON }); + assert.equal(r.status, 400, String(delta)); + assert.equal(r.body.error, 'invalid_delta'); + } + assert.equal((await adjust(env, SUPER, { user_id: FOUNDER, delta: 100_000, reason: REASON })).status, 200, 'the bound itself is allowed'); + const long = await adjust(env, SUPER, { user_id: FOUNDER, delta: 1, reason: REASON, note: 'x'.repeat(501) }); + assert.deepEqual([long.status, long.body.error], [400, 'note_too_long']); + assert.equal((await adjust(env, SUPER, { user_id: FOUNDER, delta: 1, reason: REASON, note: 'x'.repeat(500) })).status, 200); + const ref = await adjust(env, SUPER, { user_id: FOUNDER, delta: 1, reason: REASON, source_ref: 'r'.repeat(121) }); + assert.deepEqual([ref.status, ref.body.error], [400, 'invalid_source_ref']); + const nobody = await adjust(env, SUPER, { user_id: 9999, delta: 1, reason: REASON }); + assert.deepEqual([nobody.status, nobody.body.error], [404, 'user_not_found']); + const bad = await adjust(env, SUPER, { user_id: 'x', delta: 1, reason: REASON }); + assert.deepEqual([bad.status, bad.body.error], [400, 'invalid_user']); + assert.deepEqual(rowCounts(db), [2, 2, 2]); +}); + +test('a repeated source_ref is a 409 naming the existing row, of either sign, and writes nothing', async () => { + const { db, env } = setup(); + const first = await adjust(env, SUPER, { user_id: FOUNDER, delta: 40, reason: REASON, source_ref: 'support:812' }); + assert.equal(first.status, 200); + for (const delta of [40, -10]) { + const again = await adjust(env, SUPER, { user_id: FOUNDER, delta, reason: REASON, source_ref: 'support:812' }); + assert.equal(again.status, 409, `delta ${delta}`); + assert.equal(again.body.error, 'already_recorded'); + assert.equal(again.body.ledger_id, first.body.ledger_id); + assert.doesNotMatch(JSON.stringify(again.body), /UNIQUE|constraint|SQLITE/i); + } + // A rule grant's source is held too: a manual line never doubles it. + const g = await grantCredits(env, { userId: FOUNDER, ruleKey: 'bug_low', sourceRef: 'ticket:9', actor: { kind: 'system', source: 'test' } }); + const dup = await adjust(env, SUPER, { user_id: FOUNDER, delta: 5, reason: REASON, source_ref: 'ticket:9' }); + assert.deepEqual([dup.status, dup.body.ledger_id], [409, (g as any).ledger_id]); + assert.deepEqual(rowCounts(db), [2, 1, 2]); +}); + +test('a deduction past the balance is refused with the balance, and writes nothing', async () => { + const { db, env } = setup(); + await adjust(env, SUPER, { user_id: FOUNDER, delta: 20, reason: REASON }); + const r = await adjust(env, SUPER, { user_id: FOUNDER, delta: -21, reason: REASON }); + assert.deepEqual([r.status, r.body.error, r.body.balance], [400, 'would_overdraw', 20]); + assert.equal((await adjust(env, SUPER, { user_id: FOUNDER, delta: -20, reason: REASON })).body.balance, 0, 'down to zero is allowed'); + assert.deepEqual(rowCounts(db), [2, 2, 2]); +}); + +test('two deductions racing cannot both pass the balance: the check is inside the insert', async () => { + const { db, env } = setup(); + await manualAdjust(env, { userId: FOUNDER, delta: 30, reason: REASON, actor: { id: SUPER, email: 'super@example.test' } }); + const actor = { id: SUPER, email: 'super@example.test' }; + const [a, b] = await Promise.all([ + manualAdjust(env, { userId: FOUNDER, delta: -20, reason: REASON, sourceRef: 'race:a', actor }), + manualAdjust(env, { userId: FOUNDER, delta: -20, reason: REASON, sourceRef: 'race:b', actor }), + ]); + assert.deepEqual([a.status, b.status].sort(), ['recorded', 'refused']); + assert.equal(balance(db, FOUNDER), 10); +}); + +test('the Super Admin\'s grants list shows manual lines beside rule grants, marked manual, and not a revoke line', async () => { + const { env } = setup(); + const rule = await grantCredits(env, { userId: FOUNDER, ruleKey: 'bug_low', sourceRef: 'ticket:1', actor: { kind: 'system', source: 'test' } }); + const plus = await adjust(env, SUPER, { user_id: FOUNDER, delta: 50, reason: REASON }); + const minus = await adjust(env, SUPER, { user_id: FOUNDER, delta: -5, reason: REASON }); + await revokeGrant(env, { ledgerId: plus.body.ledger_id, actor: { id: SUPER, email: 'super@example.test' }, reason: 'Granted to the wrong account' }); + for (const path of ['/grants', `/grants?user_id=${FOUNDER}`]) { + const list = await call(creditRules, env, 'GET', path, SUPER); + assert.equal(list.status, 200); + const rows = list.body.grants.map((g: any) => [g.id, g.credits, g.rule_key, g.manual, g.revoked]).sort((x: any, y: any) => x[0] - y[0]); + assert.deepEqual(rows, [ + [(rule as any).ledger_id, 20, 'bug_low', false, false], + [plus.body.ledger_id, 50, 'manual', true, true], + [minus.body.ledger_id, -5, 'manual', true, false], + ], path); + } +}); diff --git a/documentation/architecture/decisions/D540.md b/documentation/architecture/decisions/D540.md index ed8195addb..671f0d9213 100644 --- a/documentation/architecture/decisions/D540.md +++ b/documentation/architecture/decisions/D540.md @@ -171,3 +171,6 @@ where each one lands. #1103's three questions above (staff, the cap, revoking spent credits) were not part of #1102 and stay built on S08's picks. + +**Later:** D608 (#1308) moves `POST /perks/admin/credits` into this service: +the Super Admin only, with a typed reason, an audit row and an inbox notice. diff --git a/documentation/architecture/decisions/D608.md b/documentation/architecture/decisions/D608.md new file mode 100644 index 0000000000..6178b4610b --- /dev/null +++ b/documentation/architecture/decisions/D608.md @@ -0,0 +1,97 @@ +## D608 — A manual credit grant or deduction is the Super Admin's, through the grant service + +**Issue #1308, slot S08, 2026-10-06.** Found by #1165's audit (area 7, +`perkGrantCredits`). No migration: the ledger already has the column this needs. + +### What was true on main + +`POST /api/perks/admin/credits` (`routes/perks.ts`) wrote a +`perk_credit_ledger` row itself: +- **Who could call it:** any admin (`requireAdmin`), not only the Super Admin. +- **What it recorded:** no reason and no audit row. +- **Who was told:** no one. The member got no inbox notice, though the `note` + showed on their ledger. +- **A reused `source_ref`:** reached the ledger's unique index as a raw + database error. +- **The grants list:** the row carried no `rule_key`, so the Super Admin's + grants list never showed it. + +That broke D540's rule that every grant goes through one service. No screen +calls the route. Its design is requested on #1165 (area 7, item 2). + +### What it does now + +The route checks the caller is the Super Admin (`requireSuperAdmin`) and +hands the rest to `manualAdjust` in `services/creditGrants.ts`: +- **The change:** a non-zero whole number of credits, at most 100,000 either + way (`MANUAL_DELTA_MAX`). That is the bound a rule's credits take in + `admin_credit_rules.ts`. A JSON string such as `"10"` is refused, not + converted. +- **The reason:** typed, at least 10 characters (`MANUAL_REASON_MIN`). That is + the floor every Super Admin credit act shares (`CREDIT_REASON_MIN`). It goes + in the audit row, not to the member. +- **The note:** what the member sees, on their ledger and in the notice. It is + at most 500 characters, and a longer one is refused rather than cut. With no + note, it reads "Credits added by the Axal team" or "Credits deducted by the + Axal team". +- **Idempotent on `source_ref`:** + - The default is `admin::`. + - A ref this member's ledger already holds, of any kind, is a 409 + `already_recorded` naming the existing row, and nothing is written. That + includes a rule grant's ref, such as `ticket:9`, so a manual line never + doubles a reward. + - The test is inside the insert, not before it. +- **Never below zero:** a deduction that would take the balance below zero is + a 400 `would_overdraw` carrying the balance. That test is also inside the + insert, so two deductions racing cannot both pass. +- **What is written:** one ledger line with `rule_key = 'manual'`. A grant is + a `grant` line and a deduction an `admin_adjust` line. A manual line is + never a bounty, so it does not count toward the monthly cap. +- **Recorded and told:** + - one `logAdminAction` row, `credit_manual_adjust`, with the member, the + change, the ledger id, the source, the reason and the note; + - one inbox notice, `credit_manual_adjust`, such as "50 credits added to + your account". A notice that fails leaves the line in place + (`notified: false`), as a rule grant does (D111). +- **Refusals:** all go through `refuse()`: + - `invalid_user`, `invalid_delta`, `reason_too_short`, `note_too_long` and + `invalid_source_ref` are 400; + - `user_not_found` is 404; + - `already_recorded` is 409; + - `would_overdraw` is 400. + +**Staff accounts.** A manual line may go to any account, staff included. D540's +"staff never earn" rule is about rewards. This is the Super Admin's own +decision, and it carries a reason. + +### The grants list + +`GET /api/admin/credit-rules/grants` now lists manual lines beside rule grants, +each with `manual: true`. A manual deduction shows as a negative `credits`. + +A revoke line is not listed as a deduction of its own. Revoking a manual +grant writes an `admin_adjust` line that also carries `rule_key = 'manual'`, +so revoke lines (`revoke:`) are left out of the list. The revoked grant +shows `revoked: true`, as before. + +A manual grant can be revoked like any grant (`revokeGrant`). A manual +deduction is undone by a manual grant. + +### Verified + +`cloudflare-worker/test/manual_credit_adjust_d608.test.ts` has 9 tests on +real SQLite, with the Perks harness's migrations read off disk and the real +routers with real JWTs. It fails on main. It covers: +- a plain admin and a founder refused with 403; +- the reason missing or short; +- the bounds on the change, the note and the source; +- a grant and a deduction, each with one ledger line, one audit row and one + inbox notice; +- a repeated `source_ref`, of either sign and against a rule grant's ref, as a + 409 that writes nothing; +- an overdraw; +- two deductions racing; +- the grants list, both with and without `?user_id=`. + +36 of 36 mutations were caught. One of them moves the balance test out of +the insert into a read before it, and the race test catches it. diff --git a/frontend/test/perks_live.test.mjs b/frontend/test/perks_live.test.mjs index 084a48b1c9..400b003687 100644 --- a/frontend/test/perks_live.test.mjs +++ b/frontend/test/perks_live.test.mjs @@ -133,9 +133,13 @@ test('the redemption code is generated, never taken from the request', () => { test('a spend can never take a balance below zero', () => { const s = read(ROUTE); // Two independent places: the claim checks affordability before batching, - // and an admin adjustment cannot push a balance negative either. + // and an admin adjustment cannot push a balance negative either. Since D608 + // that adjustment is the grant service's, with the test inside its insert. assert.match(s, /insufficient_credits/, 'the claim must refuse an unaffordable perk'); - assert.match(s, /that would take the balance below zero/, 'an admin adjustment must refuse too'); + assert.match(s, /const out = await manualAdjust\(c\.env,/, 'an admin adjustment must go through the grant service'); + const svc = read('cloudflare-worker/src/services/creditGrants.ts'); + assert.match(svc, /WHERE b\.user_id = \?\) \+ \? >= 0`/, 'an admin adjustment must refuse too, inside its insert'); + assert.match(svc, /refused\('would_overdraw'/); }); /* ---------------------------------------------------------------- *