fix(coaching): confirm before ending a reflection; pause instead of abandon (bd-2508 follow-up)

[hammad-sarfraz-1]

A slash command mid-reflection silently destroyed the rest of the coaching conversation. Now the teacher is asked, and the session pauses instead of dying.

The bug

bd-2508 closed the 269-hour trap by ending the session on any / message. That made escaping the trap and destroying the reflection the same action — no warning, no way back. Answers already given survived; every remaining question was dropped and the session was terminal.

Three outcomes now

Input	Behaviour
/menu, /help	Exempt. Session left running — the documented escape hatch gets no confirmation gate.
/lessonplan, /video, /quiz, /readingtest, /assessment	Ask first, naming the service she asked for. On YES → paused, never abandoned.
Menu picks (buttons + typed digits)	Same gate, re-applied in MenuService.
Free text	Unchanged, straight to the coach.

Plus an evening nudge (20:00–21:59 Asia/Karachi, once per pause) and a RESUME keyword.

Two traps a naive fix would walk into

1. /menu would re-trap the teacher. It sets AWAITING_MENU_CHOICE and waits on a bare 1–4. But the coaching interceptor runs ~1,000 lines before the menu handler, so it eats that digit as a reflective answer — she escapes and is instantly recaptured. Exactly what the original bd-2508 comment predicted.

Fixed with a narrow digit exemption, measured before designing it (RUMI_DB, 2026-08-04):

• 5 of 7,644 reflective answers are a bare 1–4 → costs ~1 answer in 1,500
• 365 of 7,644 are ≤ 2 chars → a looser rule would eat real answers

2. A paused session would silently self-destruct. stale-session.worker.js auto-completes conducting_conversation at 12h idle. A paused session is deliberately idle, so without an explicit exclusion the pause becomes a partial report — the same data loss this fix prevents.

The menu path needed its own gate

A slash command is visible to the coaching interceptor, which asks before the command runs. A menu pick arrives as a button/digit the interceptor deliberately defers — and that deferral is what makes /menu usable at all. So by the time MenuService runs, the ask-first opportunity has passed. guardMenuSelection() puts it back at both menu entry points.

menu_coaching is gated too: it asks for a new lesson recording, so the reflection she is already in must be paused rather than orphaned. menu_other (general chat) starts no service and is never gated.

Caught during implementation

Reading the real code caught five things a plan alone would have shipped broken:

• RailwayRedisService exposes delete(), not del() — would have thrown on every confirmation clear.
• Its get() already JSON-parses — an extra JSON.parse would have thrown on a valid payload.
• trimmedMessage is const and lowercased, so replaying a confirmed command by mutating it is impossible and would corrupt arguments. Recursion with the stashed original text instead, so /lessonplan grade 4 maths survives.
• NUM_REFLECTIVE_QUESTIONS is 1, not 4 (coaching-debrief.config.js:31). The prompt imports the constant and switches phrasing at N=1; a test greps the source to forbid a hardcoded total.
• handleMenuButtonResponse is (user, from, buttonId, language) — the first draft had the arguments in the wrong order.
• The button path deletes the awaiting_menu_selection state before dispatching, so a guard placed after it made a confirmed YES land on "that menu selection has expired". The guard now runs first, with a source-order test pinning it.

i18n

tests/setup/no-hardcoded-coaching-strings flagged the new English literals. All six strings now live in coaching-messages.js with Urdu translations, using {{placeholder}} substitution rather than ${} so they stay translatable 1:1.

An existing test asserted the bug

tests/handlers/coaching-slash-exit.test.js pinned status: 'abandoned' — the exact behaviour being removed. It is inverted rather than deleted, now asserting the pause, the confirmation, the exemption and the digit deferral, with the rationale recorded inline.

Verification

• New suite: 31 tests, proven RED before any implementation, green after. 39 including the updated slash-exit suite.
• Full suite: 148 failing suites before and after — zero regressions. Verified by stashing, not assumed. (Pre-existing failures include a missing openai module in dashboard/tests/embedder.test.js.)
• V1.0.12__coaching_pause.sql validated against real PostgreSQL 16: clean apply, idempotent re-apply, columns + partial index present, DOWN block confirmed to revert paused → abandoned and leave idx_coaching_sessions_stale intact.
• Ran the repo's cross-agent-safety checklist: imports verified, no chained multi-WHERE update, critical and best-effort writes kept separate, columns confirmed against the schema.

Schema

Three nullable columns (paused_at, pause_reason, evening_reminder_sent_at) plus a partial index. Additive; paused is a new value (no CHECK constraint on status). Also added to the 00_complete-schema.sql reconcile section per infrastructure/CLAUDE.md, or fresh installs would lack the columns.

reminder_sent_at was deliberately not reused — the 2h stale reminder owns it, and sharing would make either ping suppress the other.

Notes for review

• Nothing applied to Supabase; the migration has not been run.
• The 15-minute cron is configured in the Railway dashboard, not in any repo file — someone with access should confirm the live schedule before trusting the evening window.
• Contains V1.0.12. The companion ICT-spine PR adds V1.0.11; merging that first keeps the sequence gapless.