diff --git a/claudedocs/2026-05-14-module-split-plan.md b/claudedocs/2026-05-14-module-split-plan.md new file mode 100644 index 0000000..8f69d9c --- /dev/null +++ b/claudedocs/2026-05-14-module-split-plan.md @@ -0,0 +1,245 @@ +# vibe-mod — Module Split Plan: src/server/index.ts → routes/*.ts + +> Phase 2a 산출물. 사용자 승인 후 Phase 2b 구현. +> +> 페어: `claudedocs/2026-05-14-compose-flow-audit.md`, `docs/demo-scenario.md` + +## §0 결론 + +`src/server/index.ts` (1546줄) → 12개 파일로 분리. 라우트 분기 6개 (`compose / dashboard / undo / triggers / scheduler / settings`) + 헬퍼 4개 (`diagnostics / auth / openai / rule-validation`) + 앱 wiring (`app.ts`) + 부트스트랩 (`index.ts` 슬림). 회귀 위험 mid → low (handler signature 안 바뀜, import 경로만 바뀜). + +## §1 현재 구조 매핑 (line by line) + +| Lines | 섹션 | LOC | 이동 대상 | +|---|---|---:|---| +| 13-52 | imports + `const app = new Hono()` | 40 | `src/server/app.ts` (Hono app 생성) + per-file imports | +| 55-103 | `describeErr`, `snapshotDevvitRuntime` | 49 | `src/server/middleware/diagnostics.ts` | +| 106-178 | `isCallerModerator` | 73 | `src/server/middleware/auth.ts` | +| 182-188 | `summarizeValidationError` | 7 | `src/server/helpers/rule-validation.ts` | +| 193-202 | `isSafeRegex` | 10 | `src/server/helpers/rule-validation.ts` | +| 205-221 | `PredicateTreeShape` + `validatePredicateRegexes` | 17 | `src/server/helpers/rule-validation.ts` | +| 224-275 | compose menu route | 52 | `src/server/routes/compose.ts` | +| 277-617 | compose-rule-submit route | 341 | `src/server/routes/compose.ts` | +| 620-704 | dashboard menu route | 85 | `src/server/routes/dashboard.ts` | +| 706-766 | dashboard-action route | 61 | `src/server/routes/dashboard.ts` | +| 770-826 | undo menu route | 57 | `src/server/routes/undo.ts` | +| 830-842 | `isDuplicateTrigger` | 13 | `src/server/routes/triggers.ts` (private) | +| 844-852 | `safeParseBundle` | 9 | `src/server/helpers/rule-validation.ts` | +| 854-896 | on-post-submit trigger | 43 | `src/server/routes/triggers.ts` | +| 898-935 | on-comment-submit trigger | 38 | `src/server/routes/triggers.ts` | +| 937-947 | on-app-install trigger | 11 | `src/server/routes/triggers.ts` | +| 950-975 | seed-on-install scheduler | 26 | `src/server/routes/scheduler.ts` | +| 977-980 | on-app-upgrade trigger | 4 | `src/server/routes/triggers.ts` | +| 982-986 | on-post-report trigger | 5 | `src/server/routes/triggers.ts` | +| 987-991 | on-comment-report trigger | 5 | `src/server/routes/triggers.ts` | +| 995-1021 | audit-retention scheduler | 27 | `src/server/routes/scheduler.ts` | +| 1023-1031 | `DryRunResult` interface | 9 | `src/server/routes/scheduler.ts` (or shared) | +| 1033-1106 | dry-run-replay scheduler | 74 | `src/server/routes/scheduler.ts` | +| 1108-1154 | shadow-promote-check scheduler | 47 | `src/server/routes/scheduler.ts` | +| 1156-1204 | rate-limit-circuit-breaker scheduler | 49 | `src/server/routes/scheduler.ts` | +| 1209-1216 | validate-rate-limit settings | 8 | `src/server/routes/settings.ts` | +| 1217-1224 | validate-shadow settings | 8 | `src/server/routes/settings.ts` | +| 1228-1232 | `todayKey` | 5 | `src/server/helpers/openai.ts` (used only by compose) | +| 1234-1463 | `callOpenAI` | 230 | `src/server/helpers/openai.ts` | +| 1464-1476 | `isClarification` | 13 | `src/server/helpers/openai.ts` | +| 1478-1488 | `unwrapFormString` | 11 | `src/server/helpers/openai.ts` | +| 1491-1495 | `summarizeRule` | 5 | `src/server/helpers/openai.ts` | +| 1506-1508 | `ALLOW_GUARDED_HELP` | 3 | `src/server/helpers/openai.ts` | +| 1509 | `export default app` | 1 | `src/server/app.ts` | +| 1511-1545 | bootstrap (`if (createServer && WEBBIT_PORT)`) | 35 | `src/server/index.ts` (entry) | + +총 LOC: 1546. 분할 후 평균 파일당 ~130 LOC. + +## §2 Target file 구조 + +``` +src/server/ + index.ts # ENTRY (only the bootstrap `serve(...)`. imports app from app.ts) + app.ts # Hono app + route registrations (each routes/* exports a register function) + + middleware/ + diagnostics.ts # describeErr, snapshotDevvitRuntime + auth.ts # isCallerModerator (depends on diagnostics) + + helpers/ + rule-validation.ts # summarizeValidationError, isSafeRegex, validatePredicateRegexes, safeParseBundle + openai.ts # callOpenAI, isClarification, unwrapFormString, summarizeRule, ALLOW_GUARDED_HELP, todayKey + + routes/ + compose.ts # /internal/menu/compose-rule + /internal/form/compose-rule-submit + dashboard.ts # /internal/menu/dashboard + /internal/form/dashboard-action + undo.ts # /internal/menu/undo-action + triggers.ts # 6 trigger routes + isDuplicateTrigger + scheduler.ts # 5 scheduler routes + DryRunResult interface + settings.ts # 2 settings validators + + # Existing — unchanged + devvit-helpers.ts + evaluator.ts + evaluator.test.ts + evaluator.property.test.ts + executor.ts + executor.test.ts + executor.devvit.test.ts + fact-bag.ts + fact-bag.test.ts + routes-compose.test.ts # imports `app` — still works (re-export from app.ts via index.ts) + routes-dashboard.test.ts # same + routes-scheduler.test.ts # same + routes-settings.test.ts # same + routes-triggers.test.ts # same + routes-undo.test.ts # same +``` + +**Key invariant**: 모든 routes-*.test.ts는 `import app from './index';` 사용 중. index.ts가 app을 re-export하면 테스트 무수정 통과. + +## §3 Import graph (이동 후) + +``` +index.ts ─→ app.ts ─→ routes/{compose,dashboard,undo,triggers,scheduler,settings}.ts + │ + ├─→ middleware/auth.ts ─→ middleware/diagnostics.ts + │ + ├─→ helpers/rule-validation.ts + │ + └─→ helpers/openai.ts ─→ ../shared/system-prompt + ../shared/limits + ../shared/redis-keys + ./devvit-helpers (todayKey via dependent) +``` + +각 routes/*.ts는 해당 라우트 등록 함수를 export: +```ts +// src/server/routes/compose.ts +export function registerComposeRoutes(app: Hono): void { + app.post('/internal/menu/compose-rule', async (c) => { ... }); + app.post('/internal/form/compose-rule-submit', async (c) => { ... }); +} +``` + +`app.ts`: +```ts +import { Hono } from 'hono'; +import { registerComposeRoutes } from './routes/compose'; +import { registerDashboardRoutes } from './routes/dashboard'; +import { registerUndoRoutes } from './routes/undo'; +import { registerTriggerRoutes } from './routes/triggers'; +import { registerSchedulerRoutes } from './routes/scheduler'; +import { registerSettingsRoutes } from './routes/settings'; + +const app = new Hono(); +registerComposeRoutes(app); +registerDashboardRoutes(app); +registerUndoRoutes(app); +registerTriggerRoutes(app); +registerSchedulerRoutes(app); +registerSettingsRoutes(app); + +export default app; +``` + +`index.ts` (entry, ~30 lines): +```ts +import { serve } from '@hono/node-server'; +import { createServer, getServerPort } from '@devvit/web/server'; +import app from './app'; + +export default app; // tests still do `import app from './index'` + +if (typeof createServer === 'function' && typeof getServerPort === 'function' && process.env.WEBBIT_PORT) { + try { + serve({ fetch: app.fetch, createServer, port: getServerPort() }); + } catch (err) { + console.warn('[vibe-mod] server bootstrap skipped:', err); + } +} +``` + +## §4 이동 불가 / 주의 사항 + +| 함수 | 이슈 | 처리 | +|---|---|---| +| `app.post(...)` 등록 순서 | Hono은 등록 순서대로 처리. 분리해도 같은 순서 보장해야 함 | `app.ts`에서 register 호출 순서 = 현재 index.ts 순서 | +| `describeErr` (diagnostics) | `isCallerModerator`, 모든 route handler에서 `console.warn(..., describeErr(err))` 사용 | `middleware/diagnostics.ts`에서 export, 모든 파일에서 import | +| `snapshotDevvitRuntime` | `isCallerModerator`, `scheduler/rate-limit-circuit-breaker`에서 호출 | diagnostics.ts | +| `safeParseBundle` | dashboard, dashboard-action, scheduler/dry-run-replay, scheduler/shadow-promote-check, on-post-submit trigger, on-comment-submit trigger 6곳 사용 | helpers/rule-validation.ts | +| `todayKey` | compose-rule menu + compose-rule-submit (2곳, 같은 파일) | helpers/openai.ts (compose 의존성과 묶음) — 또는 helpers/time.ts 별도 | +| `DryRunResult` interface | scheduler/dry-run-replay 정의 + dashboard 사용 (`as DryRunResult`) | scheduler.ts에 export | +| `callOpenAI`, `isClarification`, `unwrapFormString`, `summarizeRule`, `ALLOW_GUARDED_HELP` | compose-rule-submit + (clarify form만) compose-rule menu | helpers/openai.ts (compose가 유일한 caller) | +| `Hono` type | Hono v4 인스턴스 타입 (`Hono`) | register 함수 시그니처 = `(app: Hono) => void` 단순화 OK (제네릭 안 씀) | +| `app` re-export | tests 호환성 핵심 | index.ts에서 `export default app` 유지 (re-import from app.ts) | + +## §5 분리 위험 평가 + +| 항목 | 위험도 | 완화 | +|---|---|---| +| 라우트 등록 누락 | 🔴 high | acceptance test G3 (모든 route 존재 확인) → 자동 catch | +| 핸들러 cross-import 누락 | 🟡 mid | typecheck (4 gates 1번) → 자동 catch | +| Hono middleware 순서 변경 | 🟢 low | 현재 middleware 사용 안 함 (모든 auth는 handler 내부에서 호출) | +| `import app from './index'` 깨짐 | 🟡 mid | index.ts에서 re-export, 모든 routes-*.test.ts 무수정 통과 확인 | +| ESM/CJS interop | 🟢 low | tsconfig "module": "ESNext", build = vite. 기존 패턴 그대로 | +| Devvit `createServer` 부트스트랩 누락 | 🔴 high | index.ts에서 그대로 유지 (이 부분은 코드 안 옮김) | +| 회귀 시 PR revert | 🟢 low | 단일 PR, 한 번에 revert 가능 | + +## §6 실행 순서 (Phase 2b) + +1. `git checkout -b refactor/split-server-routes` (PR #43 머지된 main 위) +2. `mkdir -p src/server/{middleware,helpers,routes}` +3. **`middleware/diagnostics.ts`** 생성 → `describeErr`, `snapshotDevvitRuntime` 이동 +4. **`middleware/auth.ts`** 생성 → `isCallerModerator` 이동 (diagnostics import) +5. **`helpers/rule-validation.ts`** 생성 → `summarizeValidationError`, `isSafeRegex`, `validatePredicateRegexes`, `PredicateTreeShape`, `safeParseBundle` 이동 +6. **`helpers/openai.ts`** 생성 → `callOpenAI`, `isClarification`, `unwrapFormString`, `summarizeRule`, `ALLOW_GUARDED_HELP`, `todayKey` 이동 +7. **`routes/compose.ts`** 생성 → 2 routes 이동, `registerComposeRoutes(app)` export +8. **`routes/dashboard.ts`** 생성 → 2 routes +9. **`routes/undo.ts`** 생성 → 1 route +10. **`routes/triggers.ts`** 생성 → 6 routes + `isDuplicateTrigger` (private) +11. **`routes/scheduler.ts`** 생성 → 5 routes + `DryRunResult` +12. **`routes/settings.ts`** 생성 → 2 routes +13. **`app.ts`** 생성 → register 6개 호출 + `export default app` +14. **`index.ts`** 슬림화 → bootstrap만 + `export { default } from './app'` (re-export) +15. **`npm run check`** → 4 gates green 확인 +16. PR 생성, CI 통과, merge + +각 단계 완료 시마다 `npm run typecheck` 실행해서 회귀 즉시 catch (snapshot 단위). + +## §7 LOC 추정 (분할 후) + +| 파일 | 예상 LOC | comment + import 포함 | +|---|---:|---| +| `index.ts` | ~30 | bootstrap only | +| `app.ts` | ~25 | wiring | +| `middleware/diagnostics.ts` | ~55 | describeErr + snapshotDevvitRuntime | +| `middleware/auth.ts` | ~80 | isCallerModerator | +| `helpers/rule-validation.ts` | ~55 | 4 helpers + PredicateTreeShape | +| `helpers/openai.ts` | ~285 | callOpenAI 큰 비중 | +| `routes/compose.ts` | ~410 | submit handler 큰 비중 | +| `routes/dashboard.ts` | ~155 | menu + action | +| `routes/undo.ts` | ~65 | menu | +| `routes/triggers.ts` | ~125 | 6 triggers + isDuplicateTrigger | +| `routes/scheduler.ts` | ~245 | 5 scheduler jobs | +| `routes/settings.ts` | ~25 | 2 validators | +| **총합** | **~1555** | (현재 1546 + register 함수 boilerplate) | + +평균 파일당 130 LOC. 가장 큰 파일 = `routes/compose.ts` 410 LOC (현재 1546 → 410 = 명확한 개선). + +## §8 Test 영향 + +- `routes-{compose,dashboard,scheduler,settings,triggers,undo}.test.ts` 6개 → 모두 `import app from './index'` → re-export로 무수정 통과 +- `evaluator.test.ts`, `executor.test.ts`, `fact-bag.test.ts` → server/index.ts 의존 안 함, 무영향 +- `executor.devvit.test.ts` → @devvit/test 사용, 영향 없음 +- 새 테스트 추가: 없음 (refactor only, 동작 동일) + +## §9 Open question (Phase 2b 시작 전 사용자 결정) + +1. **`Hono` 인스턴스 타입 제네릭** — register 함수 시그니처 `(app: Hono) => void` 단순화 vs `(app: Hono) => void` 정확형. 단순화 권장 (현재 vibe-mod는 Hono env 안 씀). +2. **`todayKey` 위치** — `helpers/openai.ts` (현재 compose에서만 사용) vs `helpers/time.ts` (미래 다른 파일이 쓸 가능성). 추천: openai.ts (YAGNI). +3. **`DryRunResult` interface 위치** — `routes/scheduler.ts` (export, dashboard에서 import) vs `shared/types.ts` (도메인 타입). 추천: scheduler.ts (single owner). +4. **단일 PR vs 단계 PR** — 1 PR로 모두 (현재 추천) vs 4-5 PR (middleware → helpers → routes/compose → routes/others). 추천: 단일 PR (한 번 review, 한 번 revert 가능). LOC 1555 + boilerplate ≈ 200 → review 부담 mid. + +## §10 References + +- 직전 phase 산출물: `claudedocs/2026-05-14-compose-flow-audit.md`, `docs/demo-scenario.md` +- PR #43 (Phase 1.6, 머지 대기 중) — 이 split의 baseline +- 사용자 직전 핸드오프: `claudedocs/2026-05-14-openai-400-resolved-handoff.md` + +작성: 2026-05-14 KST / Phase 2a diff --git a/claudedocs/2026-05-14-ux-best-practices-plan.md b/claudedocs/2026-05-14-ux-best-practices-plan.md new file mode 100644 index 0000000..41c61dc --- /dev/null +++ b/claudedocs/2026-05-14-ux-best-practices-plan.md @@ -0,0 +1,315 @@ +# vibe-mod — UX Best Practices Plan (Phase 1.7a) + +> 사용자 directive (2026-05-14): "UX는 시간 제한 두지 말고 quick이 아닌 베스트프랙티스와 최고의 사용자경험을 위해 진행." +> +> Phase 1.6 (PR #43)에서 미뤘던 deferred finding 5건 (#2 #3 #5 #7 #10) + 신규 best-practice 항목 4건을 Devvit form 제약 아래에서 재설계. Tier 1/2/3 분류 + 사용자 합의 후 Phase 1.7b 구현. + +## §0 Devvit form 제약 (best UX 설계의 ceiling) + +확인된 사항 (`@devvit/shared-types/shared/form.d.ts`): +- 필드 타입: `string` / `paragraph` / `number` / `boolean` / `select` / `image` / `group` +- 필드별 `helpText`, `disabled`, `required`, `defaultValue` +- **버튼이 필드 안에 없음** — `acceptLabel` / `cancelLabel` 만 (form 단위) +- **conditional 렌더링 불가** — 한 form은 한 번에 모두 표시 +- **form chaining 가능** — submit 응답으로 새 form 반환 +- multi-select select 가능 (`multiSelect: true`) +- `group` 필드로 시각적 묶음 + +이걸 알면 best UX 설계의 ceiling이 명확해진다 — interactive 한 화면 안에서 동적 변화는 못 하고, **form 흐름(form A → submit → form B → submit → form C)을 잘 짜는 게 best UX**. + +## §1 deferred findings 재평가 (Phase 1.6 deferred 5건) + +### Finding #2 — 결정적 JSON preview (CONFIRMATION FORM) +**현재**: 컴파일 직후 toast만 표시 (Phase 1.6에서 1줄 요약 추가). +**Best UX**: 컴파일 후 **확인 form**을 띄워서: +- 컴파일 결과를 사람이 읽을 수 있는 자연어로 다시 말해주기 + - 예: "When a post is from an account < 7 days old AND body < 50 chars, send it to mod queue." +- "원문(NL)" / "Trigger" / "조건 (When)" / "액션 (Then)" 4섹션 +- 하단: ☑ "Save as draft + run dry-run" (default ON) / Buttons: Save / Edit (back to compose with rule pre-filled) + +**왜 best**: README의 "deterministic JSON" 약속을 컴파일 직후 시각적으로 확인. 모드의 자율성 보장 (Save 전 마지막 review). + +**Devvit 제약**: form chaining으로 OK. submit 시 `confirmAction` field로 `save` / `edit` 분기. + +**Tier**: **1** (must-have for demo + best UX) + +### Finding #5 — Clarification turn limit (LOOP DETECTOR) +**현재**: turn counter 없음. LLM이 매 라운드 새 질문 → 사용자 갇힘 가능성. +**Best UX**: +- 서버측 counter (clarificationTurn field). max 3 turns. +- 3 turn 후 LLM이 또 clarification 요청하면: form 대신 toast `"I'm having trouble understanding this rule. Try rephrasing more concretely (e.g. with specific numbers like '< 7 days' or '< 50 chars')."` + "Cancel" 만 가능 +- 추가: 각 clarify form description 위에 "(Round 1 of 3)" 같은 진행 indicator + +**왜 best**: 무한 루프 방지 + 사용자에게 진행 상황 visibility. + +**Tier**: **1** + +### Finding #7 — Original rule editable in clarify modal +**현재**: original rule field가 `disabled: true` paragraph. +**Best UX**: enabled로 두되, helpText "Edit anytime — re-compile uses the latest text + your answer." +**Why best**: 사용자가 modal에서 원문도 바꿀 수 있는 자유도. 만약 input이 잘못 입력됐는데 clarify까지 왔다면 cancel 후 재시작이 아니라 inline 수정 가능. +**Tier**: **1** (small, high-affordance gain) + +### Finding #3 + #10 — Dashboard per-rule activation (INTERACTIVE DASHBOARD) +**현재**: text dump + 단일 "Activate N drafts" boolean. +**Best UX**: dashboard form을 **per-rule action panel**로 재설계: +- 상단: 요약 (active N · draft N · recent actions M · today tokens X) +- For each draft rule (max 10): + - Group field "Draft: " + - paragraph (disabled): rule.sourceNL + dry-run preview ("would match 3/20 recent posts") + - select "Action": ["Keep as draft", "Activate (shadow mode)", "Activate immediately (skip shadow)", "Delete"] +- For each active rule (max 10): + - Group "Active : " + - paragraph: sourceNL + "Activated 2h ago" + "actions: 5 shadow / 0 applied" + - select "Action": ["Keep", "Promote shadow → live", "Pause", "Delete"] +- Submit applies all selected changes in 1 atomic Redis txn. + +**왜 best**: "control surface" 패턴 (Stripe-style admin UI). per-rule visibility + per-rule action. +**Devvit 제약**: +- group 필드 사용 가능 (`type: 'group'`) +- 10 rules × 2 fields = 20 fields per form은 OK (한도 50 정도) +- 단점: form 전체가 한 화면 → 11+ rule이면 스크롤 길어짐 (현재는 max 50 rule) +**Tier**: **2** (큰 변경, 큰 임팩트, 데모에서 1 클로즈업) + +## §2 추가 best-practice 항목 (audit 외) + +### A — 빈 상태 (Empty states) +**현재**: dashboard 첫 진입 시 starter rules 5개 시드. install 직후라 사용자에겐 "Draft rules: 5" 보여서 OK. 하지만: +- starter rules가 모두 삭제된 경우 → 빈 dashboard에 "No rules yet — open the ⋯ menu → 'vibe-mod: Compose rule'" 메시지 +- compose form에 일일 quota 0/50일 때 "Welcome — write your first rule" 안내 +**Tier**: **2** + +### B — 파괴적 액션 confirm (Delete safety) +**현재**: 룰 삭제 기능 자체가 없음 (#3 추가 시 도입). 추가 시 즉시 적용은 위험. +**Best UX**: dashboard에서 "Delete" 선택 시, 다음 form에 "Confirm delete N rules" boolean. 두 단계 확인. +**Tier**: **2** (#3과 함께) + +### C — Onboarding (첫 사용자 가이드) +**현재**: install 직후 starter rules 5개 시드. 사용자는 어디서 시작할지 모름. +**Best UX**: 첫 dashboard 진입 시 (Redis flag로 first-visit 감지) 상단에 onboarding card: +- "Welcome to vibe-mod! 3 quick steps:" +- "1. We seeded 5 starter rules — see them below" +- "2. Click 'Activate' to enable any rule (shadow mode for 24h first)" +- "3. Open ⋯ → Compose rule to write your own in plain English" +- ☑ "Don't show this again" → Redis set first-visit-acknowledged +**Tier**: **3** (post-publish, 시간 압박 시 skip) + +### D — Token cost transparency +**현재**: 일일 compile counter X/50 표시 (compose form만). 누적 token 미표시. +**Best UX**: dashboard 상단 요약에 "Today: 7 compiles · 12,234 input tokens · 612 output tokens · ~$0.0006". +- 데이터는 이미 redis에 있음 (`bundle.llmTokensIn` + bundle.llmTokensOut). 화면 노출만 추가. +**Tier**: **2** + +## §3 추천 Tier (사용자 합의 대상) + +### Tier 1 (must-land for publish + best demo) — 추정 ~6h +- **#2** Compile preview confirmation form +- **#5** Clarification turn limit (3-round) + progress indicator +- **#7** Original rule editable + +### Tier 2 (should-land for best UX) — 추정 ~10h +- **#3+#10** Interactive dashboard with per-rule actions (group fields) +- **A** Empty states (dashboard + compose) +- **B** Delete confirm (with #3) +- **D** Token cost transparency + +### Tier 3 (post-publish polish) — 추정 ~6h +- **C** Onboarding card +- 추가 brainstorm 후 + +총 Tier 1+2 = ~16h 작업. D-9 (4일 = 96h) 대비 16% 시간 → 충분히 여유. + +## §4 Tier 1 implementation 스케치 + +### #2 Compile preview confirmation form + +```ts +// src/server/index.ts (will be in src/server/routes/compose.ts after Phase 2) +// After successful compile + validation, before persisting draft: + +const ruleNL = humanizeRule(validated); // new helper +return c.json({ + showForm: { + name: 'composeConfirmForm', // NEW form name → registered in devvit.json + form: { + title: 'Confirm compiled rule', + description: 'Review the rule before saving. You can edit if it doesn\'t match your intent.', + acceptLabel: 'Save + run dry-run', + cancelLabel: 'Cancel', + fields: [ + { name: 'compiledJson', label: 'Compiled rule (read-only)', type: 'paragraph', defaultValue: ruleNL, disabled: true }, + { name: 'editInsteadOfSave', label: 'Edit instead of save', type: 'boolean', defaultValue: false, helpText: 'Tick to go back to compose with the original sentence pre-filled.' }, + { name: 'rule', type: 'paragraph', label: '(internal) original NL', defaultValue: rule, disabled: true }, + { name: 'allowGuarded', type: 'boolean', label: '(internal) allowGuarded', defaultValue: !!allowGuarded, disabled: true }, + { name: 'serializedRule', type: 'paragraph', label: '(internal) compiled rule JSON', defaultValue: JSON.stringify(validated), disabled: true }, + ], + }, + }, +}); +``` + +The new `compose-confirm-submit` route reads `editInsteadOfSave`. If true, re-opens compose form with rule pre-filled. If false, persists draft + schedules dry-run (the persist+dry-run flow currently in compose-rule-submit). + +### #5 Clarification turn limit + +Add `clarificationTurn` to compose form fields (hidden as paragraph?) — actually Devvit forms don't have `hidden`. Use `disabled` paragraph with default value `'1'`: +```ts +{ name: 'clarificationTurn', label: 'Round (do not edit)', type: 'paragraph', defaultValue: '1', disabled: true } +``` +Server: read it, parse int. If LLM returns clarification AND turn >= 3: +```ts +if (isClarification(compiled) && turn >= 3) { + return c.json({ showToast: { text: "I've asked 3 clarifying questions and still can't compile this. Try rephrasing more concretely (e.g. specific numbers like '< 7 days', '< 50 chars').", appearance: 'neutral' }}); +} +``` +Else if clarification: increment turn in next form, prepend "(Round 2 of 3) " to question. + +### #7 Original rule editable + +Just remove `disabled: true` from the `name: 'rule'` paragraph in clarify form. Add helpText. + +### Test impact +- routes-compose.test.ts: add ~6 new tests (preview form rendering, edit-back, save flow, turn limit, editable original) +- 회귀 위험: low (handler signature 안 바뀜, 새 엔드포인트 추가) + +## §5 Tier 2 implementation 스케치 + +### #3+#10 Interactive dashboard + +```ts +// src/server/index.ts (current) — dashboard menu becomes: +return c.json({ + showForm: { + name: 'dashboardForm', // existing name reused, new schema + form: { + title: 'vibe-mod Dashboard', + description: `Active: ${activeCount} · Draft: ${draftCount} · Today: ${tokensIn}/${tokensOut} tokens (~$${cost.toFixed(4)})`, + acceptLabel: 'Apply changes', + cancelLabel: 'Close', + fields: [ + ...drafts.map(r => ({ + type: 'group', + label: `📝 Draft: ${r.name}`, + fields: [ + { name: `info_${r.id}`, label: 'Rule', type: 'paragraph', defaultValue: r.sourceNL, disabled: true }, + { name: `dryrun_${r.id}`, label: 'Dry-run preview', type: 'paragraph', defaultValue: dryRunSummary(r.id), disabled: true }, + { + name: `action_${r.id}`, + label: 'Action', + type: 'select', + options: [ + { label: 'Keep as draft', value: 'keep' }, + { label: 'Activate (shadow mode 24h)', value: 'activate-shadow' }, + { label: 'Activate immediately (skip shadow)', value: 'activate-now' }, + { label: 'Delete', value: 'delete' }, + ], + defaultValue: ['keep'], + }, + ], + })), + ...actives.map(r => ({ + type: 'group', + label: `${r.shadow ? '👻 Shadow' : '✅ Live'}: ${r.name}`, + fields: [ + { name: `info_${r.id}`, type: 'paragraph', label: 'Rule', defaultValue: r.sourceNL, disabled: true }, + { name: `stats_${r.id}`, type: 'paragraph', label: 'Stats', defaultValue: `${r.stats?.shadow ?? 0} shadow / ${r.stats?.applied ?? 0} applied`, disabled: true }, + { + name: `action_${r.id}`, + type: 'select', + label: 'Action', + options: [ + { label: 'Keep', value: 'keep' }, + ...(r.shadow ? [{ label: 'Promote shadow → live', value: 'promote' }] : []), + { label: 'Pause (back to shadow)', value: 'pause' }, + { label: 'Delete', value: 'delete' }, + ], + defaultValue: ['keep'], + }, + ], + })), + ], + }, + }, +}); +``` + +dashboard-action handler reads each `action_${id}` and applies. Delete actions go through Tier 2 confirm form (`dashboardConfirmDeleteForm`). + +### #B Delete confirm form + +```ts +// If any action_* === 'delete', show confirm form first: +return c.json({ + showForm: { + name: 'dashboardConfirmDeleteForm', + form: { + title: `Delete ${deleteCount} rule(s)?`, + description: deleteList.map(r => `- ${r.name}`).join('\n'), + acceptLabel: 'Confirm delete', + cancelLabel: 'Cancel', + fields: [ + { name: 'confirm', type: 'boolean', label: 'I understand this is permanent', defaultValue: false }, + ], + }, + }, +}); +``` + +### #D Token cost + +Add to dashboard summary (above): +```ts +const cost = (bundle.llmTokensIn * 0.00000015 + bundle.llmTokensOut * 0.00000060); // gpt-5.4-mini pricing +``` + +## §6 New routes / forms registered in devvit.json + +Tier 1 추가: +- `/internal/form/compose-confirm-submit` (form: composeConfirmForm) + +Tier 2 추가: +- `/internal/form/dashboard-confirm-delete` (form: dashboardConfirmDeleteForm) + +(현재 `dashboardForm` 재사용 OK. composeConfirmForm 새로 등록 필요.) + +## §7 Tier 1 vs Tier 2 trade-off (사용자 결정) + +| 옵션 | Tier 1 only | Tier 1 + Tier 2 | Tier 1 + 2 + 3 | +|---|---|---|---| +| 작업 시간 | ~6h | ~16h | ~22h | +| 데모 임팩트 | ★★★ | ★★★★ | ★★★★ | +| Best UX score | ★★ | ★★★★ | ★★★★★ | +| Risk | low | mid (큰 dashboard 변경) | mid | +| publish 마진 | ⭐ 매우 충분 | ⭐ 충분 | ⭐ 빡빡 | + +추천: **Tier 1 + Tier 2** (16h, 4일 안 OK, dashboard interactive가 README의 "audit log" 약속을 시각화). + +## §8 Phase 순서 갱신 + +1. ✅ Phase 1 — checkpoint commit +2. ✅ Phase 1.5 / 1.5b — audit + 문서 +3. ✅ Phase 1.6 — 3 quick wins (#1 #4 #6) PR #43 merged +4. ✅ Phase 2.5 / 2.5b — scenario 합의 + 문서 +5. ✅ Phase 2a — module split 설계 문서 +6. ✅ Phase 1.7a — 이 문서 +7. **next**: Phase 1.7b — Tier 1+2 구현 (사용자 합의 후) +8. Phase 2b — module split 구현 (1.7b의 새 코드 포함) +9. Phase 3 — README screenshots (1.7b 적용 UI 캡처) +10. Phase 4 — 데모 영상 자산 +11. Phase 5 — Devpost +12. **사용자 액션**: `npx devvit publish --public` (D-9 = 2026-05-18) + +## §9 Open question (사용자 결정 필요) + +1. **Tier 선택**: Tier 1만 / Tier 1+2 (추천) / Tier 1+2+3 +2. **Dashboard interactive 디자인 (#3+#10)**: group field 패턴 OK vs 별도 menu 분리 ("vibe-mod: Manage rules") — 추천: group 패턴 (한 화면 visibility 우선) +3. **Token cost (#D)** 표시 단위: $/달러 vs 토큰만 — 추천: "12,234 in / 612 out (~$0.0006 today)" 둘 다 +4. **Onboarding (#C, Tier 3)**: 시간 여유 확정 후 추가? 또는 publish 후로? + +## §10 References + +- 직전 phase 산출물: `claudedocs/2026-05-14-compose-flow-audit.md`, `docs/demo-scenario.md`, `claudedocs/2026-05-14-module-split-plan.md` +- PR #43 (Phase 1.6 완료): clarify select + ban/mute helpText + toast summary +- Devvit form 타입: `node_modules/@devvit/shared-types/shared/form.d.ts` + +작성: 2026-05-14 KST / Phase 1.7a diff --git a/devvit.json b/devvit.json index 18ab8dc..ee79c76 100644 --- a/devvit.json +++ b/devvit.json @@ -78,6 +78,13 @@ "location": ["subreddit"], "endpoint": "/internal/menu/dashboard" }, + { + "label": "vibe-mod: Manage rules", + "description": "Activate / pause / delete rules per-rule", + "forUserType": "moderator", + "location": ["subreddit"], + "endpoint": "/internal/menu/manage-rules" + }, { "label": "vibe-mod: Undo this action", "description": "Restore the post (rule-triggered removals only)", @@ -89,7 +96,10 @@ }, "forms": { "ruleComposerForm": "/internal/form/compose-rule-submit", - "dashboardForm": "/internal/form/dashboard-action" + "composeConfirmForm": "/internal/form/compose-confirm-submit", + "dashboardForm": "/internal/form/dashboard-action", + "manageRulesForm": "/internal/form/manage-rules-submit", + "manageDeleteConfirmForm": "/internal/form/manage-delete-confirm" }, "triggers": { "onPostSubmit": "/internal/trigger/on-post-submit", diff --git a/src/server/index.ts b/src/server/index.ts index a3cafcb..546931b 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -287,6 +287,7 @@ app.post('/internal/form/compose-rule-submit', async (c) => { allowGuarded: boolean; clarificationAnswer?: string | string[]; clarificationAnswerOther?: string | string[]; + clarificationTurn?: string | string[]; }>(); const rule = raw.rule; const allowGuarded = !!raw.allowGuarded; @@ -298,6 +299,12 @@ app.post('/internal/form/compose-rule-submit', async (c) => { const selectedAnswer = unwrapFormString(raw.clarificationAnswer); const clarificationAnswer = otherAnswer || selectedAnswer; + // Turn counter (audit finding #5). Round 1 on first compile, increments + // each time the LLM asks a follow-up question. Capped at MAX_CLARIFY_TURNS + // so an oscillating LLM can't trap the moderator in an infinite modal loop. + const turnRaw = unwrapFormString(raw.clarificationTurn); + const clarificationTurn = Math.max(1, Math.min(99, Number.parseInt(turnRaw, 10) || 1)); + if (!rule?.trim()) { return c.json({ showToast: { text: 'Please type a rule.', appearance: 'neutral' } }); } @@ -408,7 +415,22 @@ app.post('/internal/form/compose-rule-submit', async (c) => { // the intended option in one click instead of paraphrasing the question. // The free-text "other" field is always present as an override / escape // hatch — `compose-rule-submit` prefers it when non-empty. + // + // Turn limit (audit finding #5): if the LLM asks a question on what would + // be the (MAX_CLARIFY_TURNS + 1)-th round, refuse with an actionable toast + // instead of opening yet another modal — an oscillating model can't trap + // the moderator forever. if (isClarification(compiled)) { + if (clarificationTurn >= MAX_CLARIFY_TURNS) { + return c.json({ + showToast: { + text: `I've asked ${MAX_CLARIFY_TURNS} clarifying questions and still can't compile this rule. Try rephrasing more concretely — e.g. specific numbers like "< 7 days" or "< 50 chars".`, + appearance: 'neutral', + }, + }); + } + + const nextTurn = clarificationTurn + 1; const suggested = Array.isArray(compiled.suggestedAnswers) ? compiled.suggestedAnswers .filter((s): s is string => typeof s === 'string' && s.trim().length > 0) @@ -416,13 +438,17 @@ app.post('/internal/form/compose-rule-submit', async (c) => { .slice(0, 8) : []; + // Editable original rule (audit finding #7) — moderator can revise the + // English directly inside the clarify modal instead of cancelling out + // and starting over. The next compile uses whatever text is in the field + // PLUS the clarification answer. const fields: FormField[] = [ { name: 'rule', - label: 'Original rule (do not edit)', + label: 'Original rule (you can edit if you want to revise it)', type: 'paragraph', defaultValue: rule, - disabled: true, + helpText: 'Re-compile uses this text plus your answer below.', }, ]; @@ -459,12 +485,22 @@ app.post('/internal/form/compose-rule-submit', async (c) => { helpText: ALLOW_GUARDED_HELP, }); + // Hidden state-carrier — Devvit forms have no `hidden` type, so a disabled + // paragraph is the standard pattern for shipping state across rounds. + fields.push({ + name: 'clarificationTurn', + label: 'Round (do not edit)', + type: 'paragraph', + defaultValue: String(nextTurn), + disabled: true, + }); + return c.json({ showForm: { name: 'ruleComposerForm', form: { title: 'Clarify the rule', - description: compiled.question, + description: `(Round ${nextTurn} of ${MAX_CLARIFY_TURNS}) ${compiled.question}`, acceptLabel: 'Re-compile', fields, }, @@ -508,109 +544,214 @@ app.post('/internal/form/compose-rule-submit', async (c) => { }); } - // Append to draft bundle (sub-scoped key). All plugin RPC here is - // best-effort — see top of handler for the reddit/devvit#258 rationale. - // We've already produced a valid compiled `validated` rule above; even if - // persistence fails the user still sees the compile-success toast and the - // rule object below. - const draftKey = keys.rulesDraft(subredditName); - let draftJson: string | undefined; - try { - draftJson = (await redis.get(draftKey)) ?? undefined; - } catch (err) { - console.warn('[vibe-mod] submit: redis.get(draft) threw — starting fresh:', describeErr(err)); - } - - let llmModel = 'gpt-5.4-mini'; - try { - // Same SELECTION-array unwrap as callOpenAI (PR #39). The submit handler - // stores `llmModel` into the draft bundle as metadata; without this - // unwrap, the draft.llmModel field would hold `["gpt-5.4-mini"]` instead - // of `"gpt-5.4-mini"`, breaking later parses of the bundle. - const rawModel = await settings.get('openaiModel'); - let unwrapped: unknown = rawModel; - if (Array.isArray(rawModel) && rawModel.length > 0) unwrapped = rawModel[0]; - if (typeof unwrapped === 'string' && unwrapped.trim()) llmModel = unwrapped.trim(); - } catch (err) { - console.warn('[vibe-mod] submit: settings.get(openaiModel) threw — using default:', describeErr(err)); - } - - const draft: RuleBundleType = safeParseBundle(draftJson, 'compose/draft') ?? { - schemaVersion: '1.0.0', - bundleVersion: 0, - compiledAt: Date.now(), - llmModel, - llmTokensIn: 0, - llmTokensOut: 0, - rules: [], - }; + // Audit finding #2 — show a CONFIRMATION form before persisting. The + // moderator sees the deterministic compile result rendered as English, + // and can either Save (→ /internal/form/compose-confirm-submit, persists + + // schedules dry-run) or Edit (→ re-opens compose with the original + // sentence pre-filled). State (validated rule, tokens, model) is carried + // forward through disabled paragraph fields — Devvit forms have no + // hidden-input type, so this is the standard pattern. + const llmModel = await readOpenaiModel(); + const humanized = humanizeRule(validated); + const cost = estimateTokenCost(llmModel, tokensIn, tokensOut); + const summaryHeader = [ + `Rule name: ${validated.name}`, + `Original sentence: "${validated.sourceNL}"`, + ``, + humanized, + ``, + `Compile cost: ${tokensIn} in / ${tokensOut} out tokens (~$${cost.toFixed(5)} on ${llmModel}).`, + ].join('\n'); - const existingIdx = draft.rules.findIndex((r) => r.id === validated.id); - if (existingIdx >= 0) draft.rules[existingIdx] = validated; - else draft.rules.push(validated); + return c.json({ + showForm: { + name: 'composeConfirmForm', + form: { + title: `Confirm: "${validated.name}"`, + description: + 'Review the deterministic compile below. Save to keep this rule as a draft (with a 24h shadow period) and run a dry-run preview against recent posts. Tick "Edit instead" to go back and revise the English.', + acceptLabel: 'Save + run dry-run preview', + cancelLabel: 'Cancel', + fields: [ + { + name: 'compiledSummary', + label: 'Compiled rule (read-only)', + type: 'paragraph', + defaultValue: summaryHeader, + disabled: true, + }, + { + name: 'editInsteadOfSave', + label: 'Edit the original sentence instead of saving', + type: 'boolean', + defaultValue: false, + helpText: 'Tick to re-open the compose form with your original text pre-filled.', + }, + // ── State carriers (disabled paragraphs) ── + { name: 'rule', label: '(internal) original NL', type: 'paragraph', defaultValue: rule, disabled: true }, + { + name: 'allowGuarded', + label: '(internal) allowGuarded', + type: 'boolean', + defaultValue: !!allowGuarded, + disabled: true, + }, + { + name: 'serializedRule', + label: '(internal) compiled rule JSON', + type: 'paragraph', + defaultValue: JSON.stringify(validated), + disabled: true, + }, + { + name: 'tokensIn', + label: '(internal) tokensIn', + type: 'paragraph', + defaultValue: String(tokensIn), + disabled: true, + }, + { + name: 'tokensOut', + label: '(internal) tokensOut', + type: 'paragraph', + defaultValue: String(tokensOut), + disabled: true, + }, + { + name: 'llmModel', + label: '(internal) llmModel', + type: 'paragraph', + defaultValue: llmModel, + disabled: true, + }, + { + name: 'usingBYOK', + label: '(internal) usingBYOK', + type: 'boolean', + defaultValue: usingBYOK, + disabled: true, + }, + ], + }, + }, + }); +}); - if (draft.rules.length > 50) { - return c.json({ - showToast: { text: 'Rule cap reached (50). Delete a rule first.', appearance: 'neutral' }, - }); +// ───────────────────────────────────────────────────────────────────────────── +// Form: Compose confirm submit (audit finding #2 — Save vs Edit branch) +// ───────────────────────────────────────────────────────────────────────────── +// Persists the previously-compiled rule into the draft bundle + schedules a +// dry-run preview. If the moderator ticked "Edit instead", instead re-opens +// the compose form with their original NL pre-filled so they can revise. +app.post('/internal/form/compose-confirm-submit', async (c) => { + if (!(await isCallerModerator())) { + return c.json({ showToast: { text: 'Only moderators can use this.', appearance: 'neutral' } }); } - draft.bundleVersion += 1; - draft.compiledAt = Date.now(); - draft.llmTokensIn += tokensIn; - draft.llmTokensOut += tokensOut; - - let persisted = true; - try { - await redis.set(draftKey, JSON.stringify(draft)); - } catch (err) { - persisted = false; - console.warn('[vibe-mod] submit: redis.set(draft) threw — rule NOT persisted:', describeErr(err)); - } + const raw = await c.req.json<{ + compiledSummary?: string | string[]; + editInsteadOfSave?: boolean; + rule?: string | string[]; + allowGuarded?: boolean; + serializedRule?: string | string[]; + tokensIn?: string | string[]; + tokensOut?: string | string[]; + llmModel?: string | string[]; + usingBYOK?: boolean; + }>(); - // Increment daily compile counter (sub-scoped, BYOK skipped) — best-effort. - if (!usingBYOK) { + const rule = unwrapFormString(raw.rule); + const allowGuarded = !!raw.allowGuarded; + const serializedRule = unwrapFormString(raw.serializedRule); + const tokensIn = Math.max(0, Number.parseInt(unwrapFormString(raw.tokensIn), 10) || 0); + const tokensOut = Math.max(0, Number.parseInt(unwrapFormString(raw.tokensOut), 10) || 0); + const llmModel = unwrapFormString(raw.llmModel) || 'gpt-5.4-mini'; + const usingBYOK = !!raw.usingBYOK; + + // Edit branch — re-open compose form with the moderator's NL pre-filled + // so they can revise without retyping. We deliberately *don't* refund + // the compile we just spent (cost is real), but the moderator gets a + // second pass for free if their next compile uses cached few-shot. + if (raw.editInsteadOfSave) { + const subredditName = getCurrentSubredditName(); + let dailyCountDisplay = '—'; try { - await redis.set(todayCounterKey, String(todayCount + 1)); - await redis.expire(todayCounterKey, 86_400); + dailyCountDisplay = String(Number((await redis.get(keys.compileCount(subredditName, todayKey()))) ?? '0')); } catch (err) { - console.warn('[vibe-mod] submit: redis.set(todayCount) threw — quota not incremented:', describeErr(err)); + console.warn('[vibe-mod] confirm: redis.get(dailyCount) threw:', describeErr(err)); } + return c.json({ + showForm: { + name: 'ruleComposerForm', + form: { + title: `Edit rule for r/${subredditName}`, + description: `Compiles used today: ${dailyCountDisplay} / ${LIMITS.COMPILE_RATE_LIMIT_PER_DAY}.\nYour original text is pre-filled — revise and re-compile.`, + acceptLabel: 'Compile + Preview', + cancelLabel: 'Cancel', + fields: [ + { + name: 'rule', + label: 'Describe your rule in plain English (max 1000 characters)', + type: 'paragraph', + defaultValue: rule, + helpText: 'Edit the sentence and re-compile.', + }, + { + name: 'allowGuarded', + label: 'Allow this rule to ban/mute (otherwise removes only)', + type: 'boolean', + defaultValue: !!allowGuarded, + helpText: ALLOW_GUARDED_HELP, + }, + ], + }, + }, + }); } - // Kick off dry-run replay job — best-effort. If scheduler is unreachable - // the rule is still compiled; the user just doesn't get the dry-run preview. - let dryRunQueued = true; + // Save branch — re-validate the carried JSON (defence in depth: never + // trust round-trip state) and run the original persist + dry-run flow. + let validated: RuleType; try { - await scheduler.runJob({ - name: 'dry-run-replay', - runAt: new Date(), - data: { ruleId: validated.id, subredditName }, + validated = Rule.parse(JSON.parse(serializedRule)); + checkTreeDepth(validated.when as Parameters[0]); + validatePredicateRegexes(validated.when as PredicateTreeShape); + } catch (_err) { + return c.json({ + showToast: { + text: 'The compiled rule is no longer valid (please re-compile).', + appearance: 'neutral', + }, }); + } + + const subredditName = getCurrentSubredditName(); + const todayCounterKey = keys.compileCount(subredditName, todayKey()); + let todayCount = 0; + try { + todayCount = Number((await redis.get(todayCounterKey)) ?? '0'); } catch (err) { - dryRunQueued = false; - console.warn('[vibe-mod] submit: scheduler.runJob(dry-run) threw — no preview:', describeErr(err)); + console.warn('[vibe-mod] confirm: redis.get(todayCount) threw — skipping quota:', describeErr(err)); } - // Honest user-facing toast — say what actually happened rather than promising - // a dashboard view that won't render if persistence failed. The 1-line - // summary (audit finding #6) makes the deterministic JSON contract visible - // before the moderator opens the dashboard, and the explicit "open menu → - // View rules + log" pointer replaces the previous "check Dashboard" hint - // (Devvit toasts have no buttons, so the path has to be in the text). - const summary = summarizeRule(validated); - const lines = [`Compiled rule "${validated.name}". ${summary}.`]; - if (persisted && dryRunQueued) { - lines.push('Dry-run started — open the subreddit ⋯ menu → "vibe-mod: View rules + log" to see preview.'); - } else if (persisted) { - lines.push('Saved as draft. Open the subreddit ⋯ menu → "vibe-mod: View rules + log".'); - } else { - lines.push('Rule compiled but not persisted — plugin RPC unreachable (reddit/devvit#258).'); + const persistResult = await persistRuleAndStartDryRun({ + validated, + subredditName, + tokensIn, + tokensOut, + llmModel, + usingBYOK, + todayCount, + todayCounterKey, + }); + + if (persistResult.toast) { + return c.json({ showToast: persistResult.toast }); } return c.json({ showToast: { - text: lines.join(' '), - appearance: persisted ? 'success' : 'neutral', + text: persistResult.lines.join(' '), + appearance: persistResult.persisted ? 'success' : 'neutral', }, }); }); @@ -672,6 +813,25 @@ app.post('/internal/menu/dashboard', async (c) => { } } + // Token usage snapshot — sum across both bundles. Cost is best-effort + // (gpt-5.4-mini pricing assumed for whichever model the bundle records). + const totalIn = (active?.llmTokensIn ?? 0) + (draft?.llmTokensIn ?? 0); + const totalOut = (active?.llmTokensOut ?? 0) + (draft?.llmTokensOut ?? 0); + const llmModel = active?.llmModel ?? draft?.llmModel ?? 'gpt-5.4-mini'; + const totalCost = estimateTokenCost(llmModel, totalIn, totalOut); + + // Onboarding card (audit Tier-3 #C) — show on first dashboard visit, then + // hide once the moderator dismisses it. Best-effort Redis read; if it + // fails we fall back to "show" (better than silently hiding the intro). + let firstVisit = true; + try { + firstVisit = !(await redis.get(keys.onboardingDismissed(subredditName))); + } catch (err) { + console.warn('[vibe-mod] dashboard: redis.get(onboarding) threw:', describeErr(err)); + } + const totalRules = (active?.rules.length ?? 0) + (draft?.rules.length ?? 0); + const isEmpty = totalRules === 0 && recent.length === 0; + const summary = [ ...(rpcOk ? [] @@ -680,9 +840,22 @@ app.post('/internal/menu/dashboard', async (c) => { 'Persistence is offline; this view reflects what redis would return.', '', ]), + ...(firstVisit + ? [ + '👋 Welcome to vibe-mod. 3 quick steps:', + ' 1. We seeded 5 starter rules — see them below.', + ' 2. Open ⋯ → "vibe-mod: Manage rules" to activate one (shadow mode for 24h first).', + ' 3. Open ⋯ → "vibe-mod: Compose rule" to write your own in plain English.', + '', + ] + : []), + ...(isEmpty + ? ['No rules yet — open the subreddit ⋯ menu → "vibe-mod: Compose rule" to write your first rule.', ''] + : []), `Active rules: ${active?.rules.length ?? 0}`, `Draft rules: ${draft?.rules.length ?? 0}`, `Recent actions: ${recent.length}`, + `Tokens used (lifetime): ${totalIn.toLocaleString()} in / ${totalOut.toLocaleString()} out (~$${totalCost.toFixed(4)} on ${llmModel}).`, ...(dryRunLines.length ? ['', 'Dry-run preview (draft rules):', ...dryRunLines] : []), '', 'Recent actions:', @@ -695,76 +868,291 @@ app.post('/internal/menu/dashboard', async (c) => { form: { title: 'vibe-mod Dashboard', description: summary, - acceptLabel: draft ? `Activate ${draft.rules.length} draft rule(s)` : 'Close', - cancelLabel: 'Cancel', - fields: [{ name: 'activate', label: 'Promote draft → active', type: 'boolean', defaultValue: false }], + acceptLabel: 'Close', + cancelLabel: firstVisit ? "Don't show intro again" : 'Cancel', + fields: firstVisit + ? [ + { + name: 'dismissOnboarding', + label: 'Dismiss the welcome intro for this sub', + type: 'boolean', + defaultValue: false, + helpText: 'Tick to hide the 3-step intro on future visits.', + }, + ] + : [], }, }, }); }); +// Dashboard form submit. Now read-only (audit Tier-2 #3 + #10) — per-rule +// activation moved to the dedicated Manage rules menu. Submit handles only +// the optional onboarding-dismiss flag. app.post('/internal/form/dashboard-action', async (c) => { if (!(await isCallerModerator())) { return c.json({ showToast: { text: 'Only moderators can use this.', appearance: 'neutral' } }); } - const { activate } = await c.req.json<{ activate: boolean }>(); - if (!activate) return c.json({ showToast: 'No action taken.' }); + const { dismissOnboarding } = await c.req.json<{ dismissOnboarding?: boolean }>(); + if (dismissOnboarding) { + const subredditName = getCurrentSubredditName(); + try { + await redis.set(keys.onboardingDismissed(subredditName), '1'); + } catch (err) { + console.warn('[vibe-mod] dashboard: redis.set(onboarding) threw:', describeErr(err)); + } + return c.json({ showToast: 'Welcome intro dismissed.' }); + } + return c.json({ showToast: 'Closed.' }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Menu: Manage rules — per-rule control surface (audit findings #3 + #10) +// ───────────────────────────────────────────────────────────────────────────── +// Renders one form-group per rule with a `select` of available actions +// (Keep / Activate / Promote / Pause / Delete). Submit applies all selected +// changes in one transaction. Deletes go through a confirm form first so a +// misclick can't wipe rules silently (audit finding #B). +app.post('/internal/menu/manage-rules', async (c) => { + await c.req.json(); + if (!(await isCallerModerator())) { + return c.json({ showToast: { text: 'Only moderators can use this.', appearance: 'neutral' } }); + } const subredditName = getCurrentSubredditName(); - // Best-effort — reddit/devvit#258. Every redis touch wrapped so the user - // sees an explanatory toast rather than 500. + let active: RuleBundleType | null = null; let draft: RuleBundleType | null = null; try { - draft = safeParseBundle(await redis.get(keys.rulesDraft(subredditName)), 'activate/draft'); + active = safeParseBundle(await redis.get(keys.rulesActive(subredditName)), 'manage/active'); + draft = safeParseBundle(await redis.get(keys.rulesDraft(subredditName)), 'manage/draft'); } catch (err) { - console.warn('[vibe-mod] activate: redis.get(draft) threw:', describeErr(err)); + console.warn('[vibe-mod] manage: redis.get(rules) threw:', describeErr(err)); return c.json({ showToast: { - text: 'Plugin RPC unreachable (reddit/devvit#258). Cannot activate rules until the platform is restored.', + text: 'Plugin RPC unreachable (reddit/devvit#258). Cannot manage rules until the platform is restored.', appearance: 'neutral', }, }); } - if (!draft || draft.rules.length === 0) return c.json({ showToast: 'No draft to activate.' }); - // Stamp the activation time so the shadow-mode window is measured from when the - // rule actually went live, not from when it was compiled (a draft that sits - // for >shadowDurationHours would otherwise go live the instant it's activated). - // Carry over an existing activatedAt for a rule that's already active under the - // same id (re-activating after an edit must not reset its shadow clock). - let prevActivatedAt = new Map(); - try { - prevActivatedAt = new Map( - (safeParseBundle(await redis.get(keys.rulesActive(subredditName)), 'activate/prev-active')?.rules ?? []) - .filter((r): r is RuleType & { activatedAt: number } => typeof r.activatedAt === 'number') - .map((r) => [r.id, r.activatedAt]), - ); - } catch (err) { - console.warn('[vibe-mod] activate: redis.get(prev-active) threw — treating as none:', describeErr(err)); - } - const now = Date.now(); - for (const r of draft.rules) r.activatedAt ??= prevActivatedAt.get(r.id) ?? now; + const drafts = draft?.rules ?? []; + const actives = active?.rules ?? []; - try { - await redis.set(keys.rulesActive(subredditName), JSON.stringify(draft)); - } catch (err) { - console.warn('[vibe-mod] activate: redis.set(active) threw:', describeErr(err)); + // Empty state (audit finding #A) — guide the moderator to Compose if they + // have nothing to manage. Avoids a confusing empty modal on first run. + if (drafts.length === 0 && actives.length === 0) { return c.json({ showToast: { - text: 'Activation failed — plugin RPC unreachable (reddit/devvit#258).', + text: 'No rules yet. Open the subreddit ⋯ menu → "vibe-mod: Compose rule" to write your first rule.', appearance: 'neutral', }, }); } + + // Pre-fetch dry-run summaries so the per-rule panel can show "would match + // X/Y" inline rather than forcing the moderator to bounce to the dashboard. + // Parallelized — the previous serial loop multiplied Redis latency by the + // draft count (Gemini review #1, PR #44). With up to 50 drafts and ~10ms + // per round-trip the difference is ~500ms vs ~10ms wall time. + const dryRunByRuleId = new Map(); + const dryRunFetches = await Promise.allSettled(drafts.map((r) => redis.get(keys.dryrun(subredditName, r.id)))); + for (let i = 0; i < drafts.length; i++) { + const r = drafts[i]; + const settled = dryRunFetches[i]; + if (settled.status === 'rejected') { + console.warn(`[vibe-mod] manage: redis.get(dryrun/${r.id}) threw:`, describeErr(settled.reason)); + continue; + } + if (!settled.value) continue; + try { + const d = JSON.parse(settled.value) as DryRunResult; + if (d.status === 'ok') { + dryRunByRuleId.set( + r.id, + `Dry-run: would match ${d.matched.length}/${d.sampledPosts} recent post(s)` + + (d.matched.length ? ` → ${[...new Set(d.matched.flatMap((m) => m.would))].join(', ')}` : ''), + ); + } else { + dryRunByRuleId.set(r.id, `Dry-run: ${d.note ?? 'unavailable'}`); + } + } catch (err) { + console.warn(`[vibe-mod] manage: parse(dryrun/${r.id}) failed:`, describeErr(err)); + } + } + + const fields: FormField[] = []; + + for (const r of drafts) { + const dry = dryRunByRuleId.get(r.id) ?? 'Dry-run: pending — re-open in 30s.'; + const summary = `${r.sourceNL}\n\n${humanizeRule(r)}\n\n${dry}`; + fields.push({ + type: 'group', + label: `📝 Draft: ${r.name}`, + fields: [ + { name: `info_${r.id}`, label: 'Rule', type: 'paragraph', defaultValue: summary, disabled: true }, + { + name: `action_${r.id}`, + label: 'Action', + type: 'select', + options: [ + { label: 'Keep as draft', value: 'keep' }, + { label: 'Activate (shadow mode 24h)', value: 'activate-shadow' }, + { label: 'Activate immediately (skip shadow)', value: 'activate-now' }, + { label: 'Delete', value: 'delete' }, + ], + defaultValue: ['keep'], + multiSelect: false, + }, + ], + }); + } + + for (const r of actives) { + const status = r.shadow ? '👻 Shadow' : '✅ Live'; + const sinceMs = Date.now() - (r.activatedAt ?? r.createdAt); + const sinceHours = Math.max(0, Math.round(sinceMs / 3_600_000)); + const summary = `${r.sourceNL}\n\n${humanizeRule(r)}\n\n${status} for ~${sinceHours}h.`; + fields.push({ + type: 'group', + label: `${status}: ${r.name}`, + fields: [ + { name: `info_${r.id}`, label: 'Rule', type: 'paragraph', defaultValue: summary, disabled: true }, + { + name: `action_${r.id}`, + label: 'Action', + type: 'select', + options: [ + { label: 'Keep', value: 'keep' }, + ...(r.shadow ? [{ label: 'Promote shadow → live', value: 'promote' }] : []), + { label: 'Pause (back to draft)', value: 'pause' }, + { label: 'Delete', value: 'delete' }, + ], + defaultValue: ['keep'], + multiSelect: false, + }, + ], + }); + } + return c.json({ - showToast: { - text: 'Draft activated. Shadow mode is ON by default — promote per rule in next 24h.', - appearance: 'success', + showForm: { + name: 'manageRulesForm', + form: { + title: `Manage rules (${drafts.length} draft · ${actives.length} active)`, + description: + 'Pick an action per rule. Deletes will ask for confirmation. Activate moves a draft into the live bundle (with shadow window). Pause moves a live rule back into drafts.', + acceptLabel: 'Apply changes', + cancelLabel: 'Cancel', + fields, + }, }, }); }); +// Manage submit — collect every `action_${id}` and apply the diff atomically +// inside a single redis.set per bundle. Delete actions short-circuit into the +// confirm form so destructive intent is acknowledged once before the rules +// disappear (audit finding #B). +app.post('/internal/form/manage-rules-submit', async (c) => { + if (!(await isCallerModerator())) { + return c.json({ showToast: { text: 'Only moderators can use this.', appearance: 'neutral' } }); + } + const raw = (await c.req.json>()) || {}; + const actions: Record = {}; + for (const [k, v] of Object.entries(raw)) { + if (!k.startsWith('action_')) continue; + const id = k.slice('action_'.length); + const decision = unwrapFormString(v as string | string[]); + if (decision && decision !== 'keep') actions[id] = decision; + } + + if (Object.keys(actions).length === 0) { + return c.json({ showToast: 'No changes selected.' }); + } + + // If any deletes → confirm step first. + const deleteIds = Object.entries(actions) + .filter(([, decision]) => decision === 'delete') + .map(([id]) => id); + + if (deleteIds.length > 0) { + const subredditName = getCurrentSubredditName(); + let active: RuleBundleType | null = null; + let draft: RuleBundleType | null = null; + try { + active = safeParseBundle(await redis.get(keys.rulesActive(subredditName)), 'manage/confirm/active'); + draft = safeParseBundle(await redis.get(keys.rulesDraft(subredditName)), 'manage/confirm/draft'); + } catch (err) { + console.warn('[vibe-mod] manage-confirm: redis.get(rules) threw:', describeErr(err)); + } + const findRule = (id: string) => active?.rules.find((x) => x.id === id) ?? draft?.rules.find((x) => x.id === id); + const deleteList = deleteIds + .map((id) => { + const r = findRule(id); + return r ? `- ${r.name} (${id})` : `- ${id}`; + }) + .join('\n'); + + return c.json({ + showForm: { + name: 'manageDeleteConfirmForm', + form: { + title: `Delete ${deleteIds.length} rule(s)?`, + description: + `These rules will be removed from both the draft and active bundles. Existing audit-log entries are kept (rollback tokens for any past actions remain valid for 30 days).\n\n` + + deleteList, + acceptLabel: 'Confirm delete', + cancelLabel: 'Cancel', + fields: [ + { + name: 'confirmed', + label: 'I understand this is permanent', + type: 'boolean', + defaultValue: false, + }, + { + name: 'pendingActions', + label: '(internal) pending action map', + type: 'paragraph', + defaultValue: JSON.stringify(actions), + disabled: true, + }, + ], + }, + }, + }); + } + + // No deletes → apply non-destructive actions immediately. + const result = await applyManageActions(actions); + return c.json({ + showToast: { text: result.summary, appearance: result.persisted ? 'success' : 'neutral' }, + }); +}); + +app.post('/internal/form/manage-delete-confirm', async (c) => { + if (!(await isCallerModerator())) { + return c.json({ showToast: { text: 'Only moderators can use this.', appearance: 'neutral' } }); + } + const raw = await c.req.json<{ confirmed?: boolean; pendingActions?: string | string[] }>(); + if (!raw.confirmed) { + return c.json({ showToast: 'Delete cancelled. No rules were removed.' }); + } + let actions: Record = {}; + try { + actions = JSON.parse(unwrapFormString(raw.pendingActions)) as Record; + } catch (_err) { + return c.json({ + showToast: { text: 'Could not parse the pending action set. Re-open Manage rules.', appearance: 'neutral' }, + }); + } + const result = await applyManageActions(actions); + return c.json({ + showToast: { text: result.summary, appearance: result.persisted ? 'success' : 'neutral' }, + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // Menu: Undo (on a specific post/comment) // ───────────────────────────────────────────────────────────────────────────── @@ -1506,6 +1894,352 @@ function summarizeRule(r: RuleType): string { const ALLOW_GUARDED_HELP = "vibe-mod only emits ban/mute when your rule explicitly says 'ban' or 'mute'. This checkbox lets the compile succeed when it does — leave it off for a removes-only rule."; +// Max number of clarification rounds before the server bails with an +// actionable toast (audit finding #5). Round 1 = initial compile. Rounds +// 2-3 = LLM follow-up questions. After round 3, no more modal — the +// moderator gets advice to rephrase concretely. +const MAX_CLARIFY_TURNS = 3; + +// Pricing snapshot for token-cost display (audit finding D). gpt-5.4-mini +// is the default; prices move occasionally so this is best-effort and the +// dashboard shows it as "~$X" rather than a precise figure. Source: +// platform.openai.com/docs/pricing as of 2026-05-14. +const OPENAI_PRICING_USD_PER_TOKEN: Record = { + 'gpt-5.4-mini': { in: 0.00000015, out: 0.0000006 }, + 'gpt-5.4-nano': { in: 0.0000001, out: 0.0000004 }, + 'gpt-5.4': { in: 0.0000025, out: 0.00001 }, +}; + +// Estimate token cost for a (model, in, out) triple. Returns 0 if model +// unknown so we never crash on a future model name. +function estimateTokenCost(model: string, tokensIn: number, tokensOut: number): number { + const p = OPENAI_PRICING_USD_PER_TOKEN[model]; + if (!p) return 0; + return tokensIn * p.in + tokensOut * p.out; +} + +// Read the openaiModel SELECTION setting, defensively unwrapping the array +// that Devvit returns even for single-select (PR #39 SELECTION-array root +// cause). Returns the default model name if the setting is missing or the +// plugin RPC is unreachable. +async function readOpenaiModel(): Promise { + const DEFAULT = 'gpt-5.4-mini'; + try { + const raw = await settings.get('openaiModel'); + let unwrapped: unknown = raw; + if (Array.isArray(raw) && raw.length > 0) unwrapped = raw[0]; + if (typeof unwrapped === 'string' && unwrapped.trim()) return unwrapped.trim(); + } catch (err) { + console.warn('[vibe-mod] readOpenaiModel: settings.get threw — using default:', describeErr(err)); + } + return DEFAULT; +} + +// Apply a manage-rules action map atomically. Each id maps to one of: +// keep | activate-shadow | activate-now | promote | pause | delete +// Plain reads + 1 write per bundle. All plugin RPC wrapped because +// reddit/devvit#258 still rears its head occasionally. +async function applyManageActions(actions: Record): Promise<{ persisted: boolean; summary: string }> { + const subredditName = getCurrentSubredditName(); + let active: RuleBundleType | null = null; + let draft: RuleBundleType | null = null; + try { + active = safeParseBundle(await redis.get(keys.rulesActive(subredditName)), 'manage/apply/active'); + draft = safeParseBundle(await redis.get(keys.rulesDraft(subredditName)), 'manage/apply/draft'); + } catch (err) { + console.warn('[vibe-mod] manage/apply: redis.get(rules) threw:', describeErr(err)); + return { + persisted: false, + summary: 'Plugin RPC unreachable (reddit/devvit#258). No changes applied.', + }; + } + + // Empty bundles default — treat as no rules; deletes / promotes against + // missing rules silently no-op (the result summary tells the user). + const draftRules: RuleType[] = (draft?.rules ?? []).slice(); + const activeRules: RuleType[] = (active?.rules ?? []).slice(); + const now = Date.now(); + + let activated = 0; + let promoted = 0; + let paused = 0; + let deleted = 0; + let kept = 0; + + for (const [id, decision] of Object.entries(actions)) { + if (decision === 'keep') { + kept++; + continue; + } + const inDraftIdx = draftRules.findIndex((r) => r.id === id); + const inActiveIdx = activeRules.findIndex((r) => r.id === id); + + if (decision === 'activate-shadow' || decision === 'activate-now') { + if (inDraftIdx < 0) continue; // not a draft → no-op + const r = draftRules[inDraftIdx]; + r.shadow = decision === 'activate-shadow'; + r.activatedAt = now; + // Move from draft to active (or upsert into active by id). + const existing = activeRules.findIndex((x) => x.id === id); + if (existing >= 0) activeRules[existing] = r; + else activeRules.push(r); + draftRules.splice(inDraftIdx, 1); + activated++; + } else if (decision === 'promote') { + if (inActiveIdx < 0) continue; + const r = activeRules[inActiveIdx]; + if (r.shadow) { + r.shadow = false; + promoted++; + } + } else if (decision === 'pause') { + if (inActiveIdx < 0) continue; + const r = activeRules[inActiveIdx]; + r.shadow = true; + // Move from active to draft. ID stable. + const existing = draftRules.findIndex((x) => x.id === id); + if (existing >= 0) draftRules[existing] = r; + else draftRules.push(r); + activeRules.splice(inActiveIdx, 1); + paused++; + } else if (decision === 'delete') { + if (inDraftIdx >= 0) draftRules.splice(inDraftIdx, 1); + if (inActiveIdx >= 0) activeRules.splice(inActiveIdx, 1); + if (inDraftIdx >= 0 || inActiveIdx >= 0) deleted++; + } + } + + // Enforce the 50-rule cap on BOTH bundles before persistence (Gemini + // review #2, PR #44). pause / activate / unpause moves can push a + // bundle over the limit. Reject up front rather than persist a bundle + // that subsequent compose-rule-submit calls will refuse to extend. + if (activeRules.length > 50 || draftRules.length > 50) { + return { + persisted: false, + summary: `Rule cap exceeded after this change (active=${activeRules.length}, draft=${draftRules.length}, max=50). Delete a rule first, then retry.`, + }; + } + + // Use the currently configured model as the fallback (Gemini review #3, + // PR #44). The previous 'manage' literal broke estimateTokenCost() because + // it isn't a key in OPENAI_PRICING_USD_PER_TOKEN. We never *consume* this + // field for compile cost (manage doesn't call OpenAI), but the dashboard + // still shows "(~$X on )" using whatever the last bundle wrote. + const fallbackModel = await readOpenaiModel(); + const draftBundle: RuleBundleType = { + schemaVersion: '1.0.0', + bundleVersion: (draft?.bundleVersion ?? 0) + 1, + compiledAt: now, + llmModel: draft?.llmModel ?? fallbackModel, + llmTokensIn: draft?.llmTokensIn ?? 0, + llmTokensOut: draft?.llmTokensOut ?? 0, + rules: draftRules, + }; + const activeBundle: RuleBundleType = { + schemaVersion: '1.0.0', + bundleVersion: (active?.bundleVersion ?? 0) + 1, + compiledAt: now, + llmModel: active?.llmModel ?? fallbackModel, + llmTokensIn: active?.llmTokensIn ?? 0, + llmTokensOut: active?.llmTokensOut ?? 0, + rules: activeRules, + }; + + // Atomic dual-write via Devvit's MULTI/EXEC (Gemini review #4, PR #44). + // The previous Promise.all of two independent set()s could leave the two + // bundles out of sync if the second write failed (e.g. a paused rule + // disappears from active without showing up in draft). watch() also + // detects concurrent modifications by another moderator's manage submit + // and aborts — the toast tells the user to retry. + let persisted = true; + let aborted = false; + try { + const txn = await redis.watch(keys.rulesActive(subredditName), keys.rulesDraft(subredditName)); + await txn.multi(); + await txn.set(keys.rulesActive(subredditName), JSON.stringify(activeBundle)); + await txn.set(keys.rulesDraft(subredditName), JSON.stringify(draftBundle)); + const result = await txn.exec(); + // exec() returns null when the WATCH detected a concurrent change. + if (result == null) aborted = true; + } catch (err) { + persisted = false; + console.warn('[vibe-mod] manage/apply: txn.exec threw:', describeErr(err)); + } + if (aborted) { + persisted = false; + } + + const parts: string[] = []; + if (activated) parts.push(`activated ${activated}`); + if (promoted) parts.push(`promoted ${promoted}`); + if (paused) parts.push(`paused ${paused}`); + if (deleted) parts.push(`deleted ${deleted}`); + if (parts.length === 0 && kept > 0) parts.push(`kept ${kept} (no other action)`); + if (parts.length === 0) parts.push('no matching rules to change'); + const summary = persisted + ? `Applied: ${parts.join(', ')}.` + : aborted + ? `Another moderator changed the rules at the same time — your changes were not applied. Re-open Manage rules and try again. (Intended: ${parts.join(', ')}.)` + : `Could not persist (plugin RPC unreachable). Intended: ${parts.join(', ')}.`; + return { persisted, summary }; +} + +// Persist a validated rule into the draft bundle and schedule a dry-run. +// Extracted from compose-rule-submit so the new compose-confirm-submit +// (audit finding #2) can re-use the exact same persistence flow without +// duplicating the best-effort plugin-RPC error handling. Returns the toast +// payload the caller should send back to the moderator. +async function persistRuleAndStartDryRun(opts: { + validated: RuleType; + subredditName: string; + tokensIn: number; + tokensOut: number; + llmModel: string; + usingBYOK: boolean; + todayCount: number; + todayCounterKey: string; +}): Promise<{ + persisted: boolean; + dryRunQueued: boolean; + lines: string[]; + toast?: { text: string; appearance: 'neutral' | 'success' }; +}> { + const { validated, subredditName, tokensIn, tokensOut, llmModel, usingBYOK, todayCount, todayCounterKey } = opts; + + const draftKey = keys.rulesDraft(subredditName); + let draftJson: string | undefined; + try { + draftJson = (await redis.get(draftKey)) ?? undefined; + } catch (err) { + console.warn('[vibe-mod] persist: redis.get(draft) threw — starting fresh:', describeErr(err)); + } + + const draft: RuleBundleType = safeParseBundle(draftJson, 'compose/draft') ?? { + schemaVersion: '1.0.0', + bundleVersion: 0, + compiledAt: Date.now(), + llmModel, + llmTokensIn: 0, + llmTokensOut: 0, + rules: [], + }; + + const existingIdx = draft.rules.findIndex((r) => r.id === validated.id); + if (existingIdx >= 0) draft.rules[existingIdx] = validated; + else draft.rules.push(validated); + + if (draft.rules.length > 50) { + return { + persisted: false, + dryRunQueued: false, + lines: [], + toast: { text: 'Rule cap reached (50). Delete a rule first.', appearance: 'neutral' }, + }; + } + + draft.bundleVersion += 1; + draft.compiledAt = Date.now(); + draft.llmTokensIn += tokensIn; + draft.llmTokensOut += tokensOut; + + let persisted = true; + try { + await redis.set(draftKey, JSON.stringify(draft)); + } catch (err) { + persisted = false; + console.warn('[vibe-mod] persist: redis.set(draft) threw — rule NOT persisted:', describeErr(err)); + } + + if (!usingBYOK) { + try { + await redis.set(todayCounterKey, String(todayCount + 1)); + await redis.expire(todayCounterKey, 86_400); + } catch (err) { + console.warn('[vibe-mod] persist: redis.set(todayCount) threw — quota not incremented:', describeErr(err)); + } + } + + let dryRunQueued = true; + try { + await scheduler.runJob({ + name: 'dry-run-replay', + runAt: new Date(), + data: { ruleId: validated.id, subredditName }, + }); + } catch (err) { + dryRunQueued = false; + console.warn('[vibe-mod] persist: scheduler.runJob(dry-run) threw — no preview:', describeErr(err)); + } + + const summary = summarizeRule(validated); + const lines = [`Compiled rule "${validated.name}". ${summary}.`]; + if (persisted && dryRunQueued) { + lines.push('Dry-run started — open the subreddit ⋯ menu → "vibe-mod: View rules + log" to see preview.'); + } else if (persisted) { + lines.push('Saved as draft. Open the subreddit ⋯ menu → "vibe-mod: View rules + log".'); + } else { + lines.push('Rule compiled but not persisted — plugin RPC unreachable (reddit/devvit#258).'); + } + + return { persisted, dryRunQueued, lines }; +} + +// Convert a compiled Rule into a human-readable English description for the +// compose-confirm form (audit finding #2). Renders trigger names, the +// PredicateTree as bulleted boolean logic, and the action list. Pure +// function — same input always renders the same output, matching the +// "deterministic" promise in README §0. +function humanizeRule(r: RuleType): string { + const triggerLabel = (t: string): string => + t === 'onPostSubmit' + ? 'a new post is submitted' + : t === 'onCommentSubmit' + ? 'a new comment is submitted' + : t === 'onPostReport' + ? 'a post is reported' + : t === 'onCommentReport' + ? 'a comment is reported' + : t; + const predicateLabel = (tree: unknown, indent = ' '): string => { + if (!tree || typeof tree !== 'object') return `${indent}(empty)`; + const t = tree as PredicateTreeShape & { value?: unknown }; + if (Array.isArray(t.all)) { + const inner = t.all.map((c) => predicateLabel(c, indent + ' ')).join('\n'); + return `${indent}ALL of:\n${inner}`; + } + if (Array.isArray(t.any)) { + const inner = t.any.map((c) => predicateLabel(c, indent + ' ')).join('\n'); + return `${indent}ANY of:\n${inner}`; + } + if (t.not) return `${indent}NOT ${predicateLabel(t.not, '').trim()}`; + // Truncate long array / object values so the confirm form's + // compiledSummary doesn't bloat to thousands of characters when a + // rule uses `op: in` against a long allowlist (CodeRabbit review, + // PR #44). + const valueStr = JSON.stringify(t.value); + const display = valueStr.length > 100 ? valueStr.slice(0, 97) + '...' : valueStr; + return `${indent}${t.fact} ${t.op} ${display}`; + }; + const actionLabel = (a: { action: string; params?: Record }): string => { + const p = a.params ?? {}; + if (a.action === 'modqueue') return `send to mod queue (note: "${p.note ?? ''}")`; + if (a.action === 'remove') return `remove (spam: ${p.spam ? 'yes' : 'no'})`; + if (a.action === 'flair') return `set flair to "${p.flairText ?? ''}"`; + if (a.action === 'lock') return `lock the thread`; + if (a.action === 'report') return `report (reason: "${p.reason ?? ''}")`; + if (a.action === 'ban') + return `BAN user${p.duration ? ` for ${p.duration}d` : ' permanently'} (reason: "${p.reason ?? ''}")`; + if (a.action === 'mute') return `MUTE user for ${p.duration ?? '?'}h`; + if (a.action === 'permaban') return `PERMABAN user (reason: "${p.reason ?? ''}")`; + return a.action; + }; + const triggers = r.on.map(triggerLabel).join(' OR '); + const conditions = predicateLabel(r.when as PredicateTreeShape); + const actions = r.then.map((a) => ' - ' + actionLabel(a)).join('\n'); + const rateLimit = r.rateLimit?.perAuthor ? `\nRate limit: ${r.rateLimit.perAuthor} per author.` : ''; + return [`When ${triggers}, IF:`, conditions, `THEN:`, actions, rateLimit].filter(Boolean).join('\n'); +} + export default app; // ───────────────────────────────────────────────────────────────────────────── diff --git a/src/server/routes-compose.test.ts b/src/server/routes-compose.test.ts index 0d93d90..b1aa0ed 100644 --- a/src/server/routes-compose.test.ts +++ b/src/server/routes-compose.test.ts @@ -42,6 +42,38 @@ const VALID_COMPILED = { then: [{ action: 'modqueue', params: { note: 'low-karma' } }], }; +/** + * Helper for the new 2-step compose flow (Phase 1.7b, audit finding #2): + * compose-rule-submit → returns showForm composeConfirmForm with state + * compose-confirm-submit → reads that state, persists draft + dry-run + * + * Tests that previously asserted "1 call → showToast success" must now do + * both steps. This helper does the round-trip and returns the final body. + */ +async function compileAndConfirm( + rule: string, + allowGuarded: boolean, + extraSubmitPayload: Record = {}, +): Promise<{ confirmFormBody: any; saveBody: any }> { + const composeRes = await call('/internal/form/compose-rule-submit', { + rule, + allowGuarded, + ...extraSubmitPayload, + }); + const confirmFormBody = await composeRes.json(); + // If we got a clarification or a toast (error), there's nothing to confirm. + if (!confirmFormBody.showForm || confirmFormBody.showForm.name !== 'composeConfirmForm') { + return { confirmFormBody, saveBody: null }; + } + // Replay the form's current values back as the confirm submission. + const fields = confirmFormBody.showForm.form.fields as Array<{ name: string; defaultValue: unknown }>; + const payload: Record = {}; + for (const f of fields) payload[f.name] = f.defaultValue; + payload.editInsteadOfSave = false; + const saveRes = await call('/internal/form/compose-confirm-submit', payload); + return { confirmFormBody, saveBody: await saveRes.json() }; +} + beforeEach(() => { // default: caller is NOT a mod; tests opt in via asMod() fakeReddit.getCurrentUser.mockResolvedValue({ id: 't2_caller', username: 'caller' }); @@ -110,10 +142,8 @@ describe('POST /internal/form/compose-rule-submit', () => { ); fakeFetch.mockResolvedValue(openaiResponse(VALID_COMPILED)); - const body = await ( - await call('/internal/form/compose-rule-submit', { rule: VALID_COMPILED.sourceNL, allowGuarded: false }) - ).json(); - expect(body.showToast.appearance).toBe('success'); + const { saveBody } = await compileAndConfirm(VALID_COMPILED.sourceNL, false); + expect(saveBody.showToast.appearance).toBe('success'); // BYOK → counter not incremented expect(await fakeRedis.get(`testsub:compile:count:${today}`)).toBe('50'); }); @@ -146,9 +176,14 @@ describe('POST /internal/form/compose-rule-submit', () => { ).json(); expect(body.showForm.name).toBe('ruleComposerForm'); expect(body.showForm.form.title).toBe('Clarify the rule'); - expect(body.showForm.form.description).toBe('What counts as low karma?'); + // Phase 1.7b (audit finding #5): clarify form description gets a + // (Round X of N) prefix so the moderator sees how many tries remain. + expect(body.showForm.form.description).toMatch(/Round 2 of 3/); + expect(body.showForm.form.description).toContain('What counts as low karma?'); const fieldNames = body.showForm.form.fields.map((f: { name: string }) => f.name); expect(fieldNames).toContain('clarificationAnswer'); + // The hidden state-carrier `clarificationTurn` is now part of the form. + expect(fieldNames).toContain('clarificationTurn'); }); // ── Phase 1.6 (audit finding #1): suggestedAnswers → select field ── @@ -254,40 +289,147 @@ describe('POST /internal/form/compose-rule-submit', () => { expect(guarded?.helpText).toMatch(/explicitly says/); }); - // ── Phase 1.6 (audit finding #6): success toast carries rule summary + menu hint ── - it('success toast includes a 1-line rule summary and the View-rules menu hint', async () => { + // ── Phase 1.7b (audit finding #2): compose-confirm-submit Edit branch ── + it('compose-confirm-submit "Edit instead" re-opens the compose form with the original NL pre-filled', async () => { asMod(); fakeSettings.get.mockImplementation(async (k: string) => (k === 'openaiApiKey' ? 'sk-dev' : undefined)); fakeFetch.mockResolvedValue(openaiResponse(VALID_COMPILED)); + + // Get to the confirm form first. + const composeRes = await call('/internal/form/compose-rule-submit', { + rule: VALID_COMPILED.sourceNL, + allowGuarded: false, + }); + const confirmForm = await composeRes.json(); + const fields = confirmForm.showForm.form.fields as Array<{ name: string; defaultValue: unknown }>; + const payload: Record = {}; + for (const f of fields) payload[f.name] = f.defaultValue; + payload.editInsteadOfSave = true; // ← user ticks Edit + + const editRes = await call('/internal/form/compose-confirm-submit', payload); + const editBody = await editRes.json(); + expect(editBody.showForm.name).toBe('ruleComposerForm'); + expect(editBody.showForm.form.title).toMatch(/Edit rule/); + const ruleField = editBody.showForm.form.fields.find((f: { name: string }) => f.name === 'rule'); + expect(ruleField.defaultValue).toBe(VALID_COMPILED.sourceNL); + // Nothing was persisted on the Edit branch. + expect(await fakeRedis.get('testsub:rules:draft')).toBeUndefined(); + }); + + // ── Phase 1.7b (audit finding #5): clarification turn limit ── + it('refuses a 4th-round clarification with an actionable toast (turn limit = 3)', async () => { + asMod(); + fakeSettings.get.mockImplementation(async (k: string) => (k === 'openaiApiKey' ? 'sk-dev' : undefined)); + // LLM keeps asking for clarification. + fakeFetch.mockResolvedValue( + openaiResponse({ + needsClarification: true, + question: 'Please clarify further', + suggestedAnswers: ['option a', 'option b'], + }), + ); + // Simulate the 3rd-round form submission (clarificationTurn=3 from previous round). const body = await ( - await call('/internal/form/compose-rule-submit', { rule: VALID_COMPILED.sourceNL, allowGuarded: false }) + await call('/internal/form/compose-rule-submit', { + rule: 'something vague', + allowGuarded: false, + clarificationTurn: '3', + }) + ).json(); + expect(body.showToast).toBeDefined(); + expect(body.showToast.text).toMatch(/3 clarifying questions/); + expect(body.showToast.text).toMatch(/rephrasing more concretely/); + // No new form opened. + expect(body.showForm).toBeUndefined(); + }); + + it('clarify form bumps the turn counter on each round', async () => { + asMod(); + fakeSettings.get.mockImplementation(async (k: string) => (k === 'openaiApiKey' ? 'sk-dev' : undefined)); + fakeFetch.mockResolvedValue(openaiResponse({ needsClarification: true, question: 'Q?', suggestedAnswers: ['a'] })); + // Round 1 (no clarificationTurn → server treats as 1). + const r1 = await ( + await call('/internal/form/compose-rule-submit', { rule: 'vague rule', allowGuarded: false }) + ).json(); + const turn1 = (r1.showForm.form.fields as Array<{ name: string; defaultValue: unknown }>).find( + (f) => f.name === 'clarificationTurn', + )!.defaultValue; + expect(turn1).toBe('2'); // next round + expect(r1.showForm.form.description).toMatch(/Round 2 of 3/); + + // Round 2 (echo turn=2 back). + const r2 = await ( + await call('/internal/form/compose-rule-submit', { + rule: 'vague rule', + allowGuarded: false, + clarificationTurn: '2', + }) ).json(); - expect(body.showToast.appearance).toBe('success'); - // 1-line summary like "→ post: modqueue" - expect(body.showToast.text).toMatch(/→\s*post[\w+]*:\s*modqueue/); - // Explicit menu pointer (Devvit toasts have no buttons → text path required) - expect(body.showToast.text).toContain('vibe-mod: View rules + log'); + const turn2 = (r2.showForm.form.fields as Array<{ name: string; defaultValue: unknown }>).find( + (f) => f.name === 'clarificationTurn', + )!.defaultValue; + expect(turn2).toBe('3'); + expect(r2.showForm.form.description).toMatch(/Round 3 of 3/); }); - it('compiles a valid rule → stores a draft, bumps the counter, schedules the dry-run', async () => { + // ── Phase 1.7b (audit finding #7): editable original rule in clarify modal ── + it('clarify form leaves the Original rule field enabled (audit finding #7)', async () => { + asMod(); + fakeSettings.get.mockImplementation(async (k: string) => (k === 'openaiApiKey' ? 'sk-dev' : undefined)); + fakeFetch.mockResolvedValue(openaiResponse({ needsClarification: true, question: 'Q?', suggestedAnswers: ['a'] })); + const body = await ( + await call('/internal/form/compose-rule-submit', { rule: 'edit me', allowGuarded: false }) + ).json(); + const ruleField = (body.showForm.form.fields as Array<{ name: string; disabled?: boolean }>).find( + (f) => f.name === 'rule', + )!; + expect(ruleField.disabled).toBeFalsy(); + }); + + // ── Phase 1.6 (audit finding #6): success toast carries rule summary + menu hint ── + // After Phase 1.7b, the toast comes from /compose-confirm-submit (not the + // compose-rule-submit handler), but the wording lives in + // persistRuleAndStartDryRun() so the assertions still apply to the saved toast. + it('success toast includes a 1-line rule summary and the View-rules menu hint', async () => { + asMod(); + fakeSettings.get.mockImplementation(async (k: string) => (k === 'openaiApiKey' ? 'sk-dev' : undefined)); + fakeFetch.mockResolvedValue(openaiResponse(VALID_COMPILED)); + const { saveBody } = await compileAndConfirm(VALID_COMPILED.sourceNL, false); + expect(saveBody.showToast.appearance).toBe('success'); + expect(saveBody.showToast.text).toMatch(/→\s*post[\w+]*:\s*modqueue/); + expect(saveBody.showToast.text).toContain('vibe-mod: View rules + log'); + }); + + it('compiles a valid rule → shows confirm form → on Save: stores draft, bumps counter, schedules dry-run', async () => { asMod(); fakeSettings.get.mockImplementation(async (k: string) => k === 'openaiApiKey' ? 'sk-dev' : k === 'openaiModel' ? 'gpt-5.4-nano' : undefined, ); fakeFetch.mockResolvedValue(openaiResponse(VALID_COMPILED)); - const res = await call('/internal/form/compose-rule-submit', { - rule: VALID_COMPILED.sourceNL, - allowGuarded: false, - }); - const body = await res.json(); - expect(body.showToast.appearance).toBe('success'); - expect(body.showToast.text).toContain('Flag low-karma posts'); + const { confirmFormBody, saveBody } = await compileAndConfirm(VALID_COMPILED.sourceNL, false); + // Phase 1.7b (audit finding #2): compile-rule-submit returns a confirm form, + // not a success toast. Persistence happens on /compose-confirm-submit. + expect(confirmFormBody.showForm.name).toBe('composeConfirmForm'); + expect(confirmFormBody.showForm.form.title).toContain('Flag low-karma posts'); + // The form embeds the compiled rule + token counts as state carriers. + const fieldsByName = Object.fromEntries( + (confirmFormBody.showForm.form.fields as Array<{ name: string; defaultValue: unknown }>).map((f) => [ + f.name, + f.defaultValue, + ]), + ); + expect(fieldsByName.serializedRule).toContain('r_low_karma_flag'); + expect(fieldsByName.llmModel).toBe('gpt-5.4-nano'); + + // After Save: draft persisted, counter bumped, dry-run scheduled. + expect(saveBody.showToast.appearance).toBe('success'); + expect(saveBody.showToast.text).toContain('Flag low-karma posts'); const draft = JSON.parse((await fakeRedis.get('testsub:rules:draft'))!); expect(draft.rules).toHaveLength(1); expect(draft.rules[0].id).toBe('r_low_karma_flag'); - expect(draft.rules[0].shadow).toBe(true); // always seeded in shadow + expect(draft.rules[0].shadow).toBe(true); expect(draft.rules[0].createdBy).toBe('t2_caller'); expect(draft.bundleVersion).toBe(1); @@ -306,9 +448,9 @@ describe('POST /internal/form/compose-rule-submit', () => { asMod(); fakeSettings.get.mockImplementation(async (k: string) => (k === 'openaiApiKey' ? 'sk-dev' : undefined)); fakeFetch.mockResolvedValue(openaiResponse(VALID_COMPILED)); - await call('/internal/form/compose-rule-submit', { rule: VALID_COMPILED.sourceNL, allowGuarded: false }); + await compileAndConfirm(VALID_COMPILED.sourceNL, false); fakeFetch.mockResolvedValue(openaiResponse({ ...VALID_COMPILED, name: 'Flag low-karma posts (v2)' })); - await call('/internal/form/compose-rule-submit', { rule: VALID_COMPILED.sourceNL, allowGuarded: false }); + await compileAndConfirm(VALID_COMPILED.sourceNL, false); const draft = JSON.parse((await fakeRedis.get('testsub:rules:draft'))!); expect(draft.rules).toHaveLength(1); @@ -326,17 +468,20 @@ describe('POST /internal/form/compose-rule-submit', () => { }; fakeFetch.mockResolvedValue(openaiResponse(banRule)); - const blocked = await ( - await call('/internal/form/compose-rule-submit', { rule: 'ban repeat spammers', allowGuarded: false }) - ).json(); + // First attempt — ban without "Allow ban/mute" → blocked at compose-rule-submit + // (toast, no confirm form). Use the raw call() so we can assert the toast. + const blockedRes = await call('/internal/form/compose-rule-submit', { + rule: 'ban repeat spammers', + allowGuarded: false, + }); + const blocked = await blockedRes.json(); expect(blocked.showToast.text).toMatch(/ban\/mute/i); expect(await fakeRedis.get('testsub:rules:draft')).toBeUndefined(); + // Second attempt — with allowGuarded: true → goes through confirm + save. fakeFetch.mockResolvedValue(openaiResponse(banRule)); - const allowed = await ( - await call('/internal/form/compose-rule-submit', { rule: 'ban repeat spammers', allowGuarded: true }) - ).json(); - expect(allowed.showToast.appearance).toBe('success'); + const { saveBody } = await compileAndConfirm('ban repeat spammers', true); + expect(saveBody.showToast.appearance).toBe('success'); const draft = JSON.parse((await fakeRedis.get('testsub:rules:draft'))!); expect(draft.rules[0].then[0].action).toBe('ban'); }); @@ -385,10 +530,8 @@ describe('POST /internal/form/compose-rule-submit', () => { }); fakeFetch.mockResolvedValue(openaiResponse(VALID_COMPILED)); - const body = await ( - await call('/internal/form/compose-rule-submit', { rule: VALID_COMPILED.sourceNL, allowGuarded: false }) - ).json(); - expect(body.showToast.appearance).toBe('success'); + const { saveBody } = await compileAndConfirm(VALID_COMPILED.sourceNL, false); + expect(saveBody.showToast.appearance).toBe('success'); // confirm we DID call OpenAI (BYOK throw didn't fatal the flow) expect(fakeFetch).toHaveBeenCalled(); }); @@ -443,21 +586,15 @@ describe('POST /internal/form/compose-rule-submit', () => { it('returns a deterministic fake compiled rule when VIBE_MOD_AI_PROVIDER=mock (local-only)', async () => { asMod(); - // Mock provider bypasses settings AND fetch entirely. - fakeSettings.get.mockImplementation(async () => { - throw new Error('should not be called'); - }); + // Mock provider bypasses settings AND fetch entirely (for the compile call). + // The confirm flow still reads openaiModel for the cost display, so allow that. + fakeSettings.get.mockImplementation(async (k: string) => (k === 'openaiModel' ? 'gpt-5.4-mini' : undefined)); const prev = process.env.VIBE_MOD_AI_PROVIDER; process.env.VIBE_MOD_AI_PROVIDER = 'mock'; try { - const body = await ( - await call('/internal/form/compose-rule-submit', { - rule: 'flag brand-new accounts within 72h', - allowGuarded: false, - }) - ).json(); - expect(body.showToast.appearance).toBe('success'); - expect(body.showToast.text).toMatch(/Mock compiled rule/); + const { saveBody } = await compileAndConfirm('flag brand-new accounts within 72h', false); + expect(saveBody.showToast.appearance).toBe('success'); + expect(saveBody.showToast.text).toMatch(/Mock compiled rule/); expect(fakeFetch).not.toHaveBeenCalled(); } finally { if (prev === undefined) delete process.env.VIBE_MOD_AI_PROVIDER; diff --git a/src/server/routes-dashboard.test.ts b/src/server/routes-dashboard.test.ts index 1e68338..894c690 100644 --- a/src/server/routes-dashboard.test.ts +++ b/src/server/routes-dashboard.test.ts @@ -1,7 +1,10 @@ // src/server/routes-dashboard.test.ts -// Functional call-tests for the Dashboard feature: the menu that renders the -// rules + recent-actions summary, and the form action that promotes a draft -// bundle to active. +// Functional call-tests for the Dashboard feature. +// +// Phase 1.7b (audit Tier-2 #3 + #10): Dashboard is now read-only — per-rule +// activation moved to the dedicated "Manage rules" menu (see +// routes-manage.test.ts). The Dashboard form's submit only handles the +// optional onboarding-dismiss flag (Tier-3 #C). import { describe, it, expect } from 'vitest'; import app from './index'; @@ -45,13 +48,22 @@ describe('POST /internal/menu/dashboard', () => { expect(body.showForm.form.acceptLabel).toBe('Close'); }); - it('summarises active + draft counts and recent actions, with an Activate label when a draft exists', async () => { + it('summarises active + draft counts, recent actions, and lifetime token cost (Tier-2 #D)', async () => { asMod(); await fakeRedis.set( 'testsub:rules:active', - JSON.stringify({ ...seedStarterRules(1), rules: seedStarterRules(1).rules.slice(0, 2) }), + JSON.stringify({ + ...seedStarterRules(1), + rules: seedStarterRules(1).rules.slice(0, 2), + llmTokensIn: 1000, + llmTokensOut: 200, + llmModel: 'gpt-5.4-mini', + }), + ); + await fakeRedis.set( + 'testsub:rules:draft', + JSON.stringify({ ...seedStarterRules(2), llmTokensIn: 500, llmTokensOut: 100 }), ); - await fakeRedis.set('testsub:rules:draft', JSON.stringify(seedStarterRules(2))); await seedAudit( 'a_1', { action: 'modqueue', outcome: 'applied', ruleSourceNL: 'flag low karma posts', thingId: 't3_x' }, @@ -71,7 +83,9 @@ describe('POST /internal/menu/dashboard', () => { expect(body.showForm.form.description).toContain('Recent actions: 2'); expect(body.showForm.form.description).toContain('modqueue (applied)'); expect(body.showForm.form.description).toContain('remove (shadow)'); - expect(body.showForm.form.acceptLabel).toBe('Activate 5 draft rule(s)'); + expect(body.showForm.form.description).toMatch(/Tokens used \(lifetime\): 1,500 in \/ 300 out/); + // Dashboard no longer triggers activation — that moved to Manage rules. + expect(body.showForm.form.acceptLabel).toBe('Close'); }); it('shows the dry-run preview for draft rules that have a stored result', async () => { @@ -112,37 +126,58 @@ describe('POST /internal/menu/dashboard', () => { }); }); -describe('POST /internal/form/dashboard-action', () => { +// Phase 1.7b — dashboard is now read-only (audit Tier-2 #3 + #10). +// The "Activate draft" flow lives in the new Manage rules menu now +// (covered by routes-manage.test.ts in this same PR). +describe('POST /internal/form/dashboard-action (read-only dashboard, Phase 1.7b)', () => { it('rejects a non-moderator', async () => { - const body = await (await call('/internal/form/dashboard-action', { activate: true })).json(); + const body = await (await call('/internal/form/dashboard-action', { dismissOnboarding: true })).json(); expect(body.showToast).toEqual({ text: 'Only moderators can use this.', appearance: 'neutral' }); }); - it('does nothing when the activate checkbox is off', async () => { + it('returns "Closed." when no flag is set', async () => { asMod(); - const body = await (await call('/internal/form/dashboard-action', { activate: false })).json(); - expect(body.showToast).toBe('No action taken.'); + const body = await (await call('/internal/form/dashboard-action', {})).json(); + expect(body.showToast).toBe('Closed.'); }); - it('reports when there is no draft to activate', async () => { + it('persists the onboarding-dismissed flag when ticked (Tier-3 #C)', async () => { asMod(); - const body = await (await call('/internal/form/dashboard-action', { activate: true })).json(); - expect(body.showToast).toBe('No draft to activate.'); + const body = await (await call('/internal/form/dashboard-action', { dismissOnboarding: true })).json(); + expect(body.showToast).toBe('Welcome intro dismissed.'); + expect(await fakeRedis.get('testsub:onboarding:dismissed')).toBe('1'); }); +}); - it('promotes the draft bundle to active and stamps activatedAt on each rule', async () => { +describe('Dashboard onboarding + empty state (Phase 1.7b Tier-3 #C, Tier-2 #A)', () => { + it('shows the welcome card on first visit (no Redis flag)', async () => { asMod(); - const seeded = seedStarterRules(123); - await fakeRedis.set('testsub:rules:draft', JSON.stringify(seeded)); - - const body = await (await call('/internal/form/dashboard-action', { activate: true })).json(); - expect(body.showToast.appearance).toBe('success'); - expect(body.showToast.text).toMatch(/Shadow mode is ON/i); - - const active = JSON.parse((await fakeRedis.get('testsub:rules:active')) ?? '{}') as { - rules: Array<{ id: string; activatedAt?: number }>; - }; - expect(active.rules.map((r) => r.id)).toEqual(seeded.rules.map((r) => r.id)); - for (const r of active.rules) expect(typeof r.activatedAt).toBe('number'); + const body = await ( + await call('/internal/menu/dashboard', { location: 'subreddit', targetId: 't5_testsub' }) + ).json(); + expect(body.showForm.form.description).toContain('Welcome to vibe-mod'); + expect(body.showForm.form.description).toContain('3 quick steps'); + const fieldNames = body.showForm.form.fields.map((f: { name: string }) => f.name); + expect(fieldNames).toContain('dismissOnboarding'); + }); + + it('hides the welcome card once dismissed', async () => { + asMod(); + await fakeRedis.set('testsub:onboarding:dismissed', '1'); + const body = await ( + await call('/internal/menu/dashboard', { location: 'subreddit', targetId: 't5_testsub' }) + ).json(); + expect(body.showForm.form.description).not.toContain('Welcome to vibe-mod'); + expect(body.showForm.form.fields).toEqual([]); + }); + + it('emits a clear empty state when there are zero rules and zero recent actions (Tier-2 #A)', async () => { + asMod(); + await fakeRedis.set('testsub:onboarding:dismissed', '1'); + const body = await ( + await call('/internal/menu/dashboard', { location: 'subreddit', targetId: 't5_testsub' }) + ).json(); + expect(body.showForm.form.description).toContain('No rules yet'); + expect(body.showForm.form.description).toContain('vibe-mod: Compose rule'); }); }); diff --git a/src/server/routes-manage.test.ts b/src/server/routes-manage.test.ts new file mode 100644 index 0000000..5c10eef --- /dev/null +++ b/src/server/routes-manage.test.ts @@ -0,0 +1,265 @@ +// src/server/routes-manage.test.ts +// Functional call-tests for the Manage rules feature (Phase 1.7b, audit +// findings #3 + #10): per-rule control surface for activating, promoting, +// pausing, and deleting rules. Replaces the previous bulk-activate flow +// that lived on the Dashboard form. +// +// Flow: +// /internal/menu/manage-rules → renders one group per rule with a select +// /internal/form/manage-rules-submit → applies non-destructive actions OR +// forwards deletes to the confirm form +// /internal/form/manage-delete-confirm → applies once explicitly confirmed + +import { describe, it, expect } from 'vitest'; +import app from './index'; +import { fakeRedis, fakeReddit, fakeListing } from '../../test/setup'; +import { seedStarterRules } from '../shared/starter-rules'; + +const call = (path: string, body: unknown = {}) => + app.fetch( + new Request(`http://localhost${path}`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify(body), + }), + ); + +function asMod() { + fakeReddit.getModerators.mockResolvedValue(fakeListing([{ username: 'caller' }])); +} + +describe('POST /internal/menu/manage-rules', () => { + it('rejects a non-moderator', async () => { + const body = await ( + await call('/internal/menu/manage-rules', { location: 'subreddit', targetId: 't5_testsub' }) + ).json(); + expect(body.showToast).toEqual({ text: 'Only moderators can use this.', appearance: 'neutral' }); + }); + + it('shows an empty-state toast when there are no rules at all', async () => { + asMod(); + const body = await ( + await call('/internal/menu/manage-rules', { location: 'subreddit', targetId: 't5_testsub' }) + ).json(); + expect(body.showToast.text).toMatch(/No rules yet/); + expect(body.showToast.text).toContain('vibe-mod: Compose rule'); + }); + + it('renders one group per draft rule with the expected action options', async () => { + asMod(); + const seeded = seedStarterRules(123); + await fakeRedis.set('testsub:rules:draft', JSON.stringify(seeded)); + + const body = await ( + await call('/internal/menu/manage-rules', { location: 'subreddit', targetId: 't5_testsub' }) + ).json(); + expect(body.showForm.name).toBe('manageRulesForm'); + const groups = body.showForm.form.fields.filter((f: { type: string }) => f.type === 'group'); + expect(groups.length).toBe(seeded.rules.length); + const firstGroup = groups[0]; + const actionField = firstGroup.fields.find((f: { name: string }) => f.name.startsWith('action_')) as { + options: Array<{ value: string }>; + }; + expect(actionField.options.map((o) => o.value)).toEqual(['keep', 'activate-shadow', 'activate-now', 'delete']); + }); + + it('renders a "Promote shadow → live" option only for shadow active rules', async () => { + asMod(); + const liveRule = { + ...seedStarterRules(1).rules[0], + activatedAt: 1, + shadow: false, + }; + const shadowRule = { + ...seedStarterRules(2).rules[1], + activatedAt: 1, + shadow: true, + }; + await fakeRedis.set( + 'testsub:rules:active', + JSON.stringify({ ...seedStarterRules(1), rules: [liveRule, shadowRule] }), + ); + + const body = await ( + await call('/internal/menu/manage-rules', { location: 'subreddit', targetId: 't5_testsub' }) + ).json(); + const groups = body.showForm.form.fields as Array<{ + type: string; + label: string; + fields: Array<{ name: string; options?: Array<{ value: string }> }>; + }>; + const liveGroup = groups.find((g) => g.label.startsWith('✅ Live'))!; + const liveActions = liveGroup.fields.find((f) => f.name.startsWith('action_'))!.options!.map((o) => o.value); + expect(liveActions).toEqual(['keep', 'pause', 'delete']); // no `promote` for already-live + const shadowGroup = groups.find((g) => g.label.startsWith('👻 Shadow'))!; + const shadowActions = shadowGroup.fields.find((f) => f.name.startsWith('action_'))!.options!.map((o) => o.value); + expect(shadowActions).toContain('promote'); + }); +}); + +describe('POST /internal/form/manage-rules-submit', () => { + it('rejects a non-moderator', async () => { + const body = await (await call('/internal/form/manage-rules-submit', {})).json(); + expect(body.showToast).toEqual({ text: 'Only moderators can use this.', appearance: 'neutral' }); + }); + + it('says "No changes selected" when every action is "keep"', async () => { + asMod(); + const body = await ( + await call('/internal/form/manage-rules-submit', { + action_r_a: ['keep'], + action_r_b: ['keep'], + }) + ).json(); + expect(body.showToast).toBe('No changes selected.'); + }); + + it('applies activate-shadow: moves a draft to active with shadow=true and stamps activatedAt', async () => { + asMod(); + const seeded = seedStarterRules(100); + await fakeRedis.set('testsub:rules:draft', JSON.stringify(seeded)); + const targetId = seeded.rules[0].id; + + const body = await ( + await call('/internal/form/manage-rules-submit', { + [`action_${targetId}`]: ['activate-shadow'], + }) + ).json(); + expect(body.showToast.appearance).toBe('success'); + expect(body.showToast.text).toMatch(/activated 1/); + + const draft = JSON.parse((await fakeRedis.get('testsub:rules:draft'))!); + const active = JSON.parse((await fakeRedis.get('testsub:rules:active'))!); + expect(draft.rules.map((r: { id: string }) => r.id)).not.toContain(targetId); + const moved = active.rules.find((r: { id: string }) => r.id === targetId); + expect(moved).toBeDefined(); + expect(moved.shadow).toBe(true); + expect(typeof moved.activatedAt).toBe('number'); + }); + + it('applies activate-now: shadow=false, immediately live', async () => { + asMod(); + const seeded = seedStarterRules(101); + await fakeRedis.set('testsub:rules:draft', JSON.stringify(seeded)); + const targetId = seeded.rules[0].id; + + await call('/internal/form/manage-rules-submit', { [`action_${targetId}`]: ['activate-now'] }); + + const active = JSON.parse((await fakeRedis.get('testsub:rules:active'))!); + const moved = active.rules.find((r: { id: string }) => r.id === targetId); + expect(moved.shadow).toBe(false); + }); + + it('applies promote: shadow active → live, no movement between bundles', async () => { + asMod(); + const seeded = seedStarterRules(102).rules[0]; + await fakeRedis.set( + 'testsub:rules:active', + JSON.stringify({ ...seedStarterRules(102), rules: [{ ...seeded, shadow: true, activatedAt: 1 }] }), + ); + await call('/internal/form/manage-rules-submit', { [`action_${seeded.id}`]: ['promote'] }); + const active = JSON.parse((await fakeRedis.get('testsub:rules:active'))!); + expect(active.rules[0].shadow).toBe(false); + }); + + it('applies pause: live active → draft (shadow=true)', async () => { + asMod(); + const seeded = seedStarterRules(103).rules[0]; + await fakeRedis.set( + 'testsub:rules:active', + JSON.stringify({ ...seedStarterRules(103), rules: [{ ...seeded, shadow: false, activatedAt: 1 }] }), + ); + await call('/internal/form/manage-rules-submit', { [`action_${seeded.id}`]: ['pause'] }); + const active = JSON.parse((await fakeRedis.get('testsub:rules:active'))!); + const draft = JSON.parse((await fakeRedis.get('testsub:rules:draft'))!); + expect(active.rules.find((r: { id: string }) => r.id === seeded.id)).toBeUndefined(); + const movedBack = draft.rules.find((r: { id: string }) => r.id === seeded.id); + expect(movedBack.shadow).toBe(true); + }); + + it('forwards delete actions to the confirm form (audit Tier-2 #B)', async () => { + asMod(); + const seeded = seedStarterRules(104); + await fakeRedis.set('testsub:rules:draft', JSON.stringify(seeded)); + const targetA = seeded.rules[0].id; + const targetB = seeded.rules[1].id; + const body = await ( + await call('/internal/form/manage-rules-submit', { + [`action_${targetA}`]: ['delete'], + [`action_${targetB}`]: ['delete'], + [`action_${seeded.rules[2].id}`]: ['keep'], + }) + ).json(); + expect(body.showForm.name).toBe('manageDeleteConfirmForm'); + expect(body.showForm.form.title).toBe('Delete 2 rule(s)?'); + // Pending action map must round-trip exactly to the confirm endpoint. + const pending = body.showForm.form.fields.find((f: { name: string }) => f.name === 'pendingActions'); + const parsed = JSON.parse(pending.defaultValue); + expect(parsed).toEqual({ [targetA]: 'delete', [targetB]: 'delete' }); + }); +}); + +describe('POST /internal/form/manage-delete-confirm', () => { + it('cancels when confirm is unchecked', async () => { + asMod(); + const body = await ( + await call('/internal/form/manage-delete-confirm', { + confirmed: false, + pendingActions: '{"r_x":"delete"}', + }) + ).json(); + expect(body.showToast).toBe('Delete cancelled. No rules were removed.'); + }); + + it('deletes from both bundles when confirmed', async () => { + asMod(); + const seeded = seedStarterRules(200); + await fakeRedis.set('testsub:rules:draft', JSON.stringify(seeded)); + const targetA = seeded.rules[0].id; + const body = await ( + await call('/internal/form/manage-delete-confirm', { + confirmed: true, + pendingActions: JSON.stringify({ [targetA]: 'delete' }), + }) + ).json(); + expect(body.showToast.appearance).toBe('success'); + expect(body.showToast.text).toMatch(/deleted 1/); + const draft = JSON.parse((await fakeRedis.get('testsub:rules:draft'))!); + expect(draft.rules.find((r: { id: string }) => r.id === targetA)).toBeUndefined(); + }); + + // Phase 1.7c (Gemini review #2 on PR #44): cap enforcement on apply. + it('refuses to persist when an action would push a bundle over the 50-rule cap', async () => { + asMod(); + // Pre-load active with 50 rules (the cap). Try to pause a draft into it. + const seeded = seedStarterRules(900); + const cap50 = Array.from({ length: 50 }, (_v, i) => ({ ...seeded.rules[0], id: `r_cap_${i}` })); + await fakeRedis.set('testsub:rules:active', JSON.stringify({ ...seeded, rules: cap50 })); + await fakeRedis.set( + 'testsub:rules:draft', + JSON.stringify({ ...seeded, rules: [{ ...seeded.rules[0], id: 'r_extra' }] }), + ); + // activate-shadow → would push active to 51. + const body = await ( + await call('/internal/form/manage-rules-submit', { action_r_extra: ['activate-shadow'] }) + ).json(); + expect(body.showToast.appearance).toBe('neutral'); + expect(body.showToast.text).toMatch(/Rule cap exceeded/); + expect(body.showToast.text).toMatch(/active=51/); + // Original active bundle untouched. + const active = JSON.parse((await fakeRedis.get('testsub:rules:active'))!); + expect(active.rules).toHaveLength(50); + }); + + it('returns a friendly error when the carried action map is malformed', async () => { + asMod(); + const body = await ( + await call('/internal/form/manage-delete-confirm', { + confirmed: true, + pendingActions: '{not json}', + }) + ).json(); + expect(body.showToast.appearance).toBe('neutral'); + expect(body.showToast.text).toMatch(/Could not parse/); + }); +}); diff --git a/src/shared/redis-keys.ts b/src/shared/redis-keys.ts index 94072ca..5512751 100644 --- a/src/shared/redis-keys.ts +++ b/src/shared/redis-keys.ts @@ -50,6 +50,9 @@ export const keys = { /** Trigger-dedupe sentinel: this {trigger, thingId} pair was already handled. */ seen: (sub: string, trigger: string, thingId: string) => `${sub}:seen:${trigger}:${thingId}`, + + /** "1" if the moderator dismissed the dashboard onboarding intro for this sub. */ + onboardingDismissed: (sub: string) => `${sub}:onboarding:dismissed`, } as const; /** Global keys that are NOT subreddit-scoped (rare). */ diff --git a/test/devvit-testkit.ts b/test/devvit-testkit.ts index e99c539..3b0b869 100644 --- a/test/devvit-testkit.ts +++ b/test/devvit-testkit.ts @@ -40,7 +40,12 @@ export interface FakeRedis { export interface FakeTxn { multi: () => Promise; discard: () => Promise; - exec: () => Promise; + // Real Devvit Redis returns an array of per-command results on success and + // `null` when WATCH detected a concurrent modification (matching ioredis / + // node-redis MULTI/EXEC semantics). Mirror that so production code that + // checks `result == null` to detect a contended write doesn't get a false + // abort signal in tests. + exec: () => Promise; get: (k: string) => Promise; set: (k: string, v: string) => Promise; expire: (k: string, ttl: number) => Promise; @@ -106,10 +111,14 @@ export function makeFakeRedis(): FakeRedis { }, }; - const watch = async (_k: string): Promise => ({ + const watch = async (..._keys: string[]): Promise => ({ multi: async () => {}, discard: async () => {}, - exec: async () => {}, + // No concurrent-modification simulation in the fake — every exec succeeds + // with an empty result array (mirroring real Redis "no per-command + // results to report"). Tests that need to assert WATCH abort behaviour + // can override this on a per-call basis with vi.spyOn. + exec: async () => [], get: base.get, set: base.set, expire: base.expire,