Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ROADMAP.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Omi Support Bot Roadmap

## October 23: #66 shutdown drain evidence — code side only

Issue #66's code side is addressed on this draft branch (rebased onto current main `d108e1a`, after #67 and #74 merged): `SupportRuntime.drain()` now logs one privacy-safe line per accepted case at shutdown (`finished=`, `returned unstarted request`, `left for reclaim … delivery=not_started|delivery=uncertain`, `unstarted return failed`), so a bounded deployment log can distinguish a completed drain from returned unstarted work, an abandoned-work reclaim and uncertain delivery. **Diagnosis: no runtime defect was found in code** — the drain path already lets started work finish inside the budget, returns unstarted work at the timeout instead of leaving it to expire as abandoned, and never replays delivery-started work; the October 19 evidence gap was the absence of per-case lines in a bounded log window. The branch adds the missing evidence, not longer waits or replayed work, and preserves the merged ownership-error hardening and fenced defer. Three new tests fail on main and pass on the branch; a fourth pins the existing no-replay-after-delivery fence. Full local suite and offline eval pass on the rebased head. **Rollout pending:** the branch is unmerged/undeployed. The controlled same-commit Redeploy drain smoke on the deployed main **passed** (clean drain, one post-SIGTERM reply); this branch's per-case lines would make the *next* teardown's evidence readable at per-case granularity, but they are not yet live and are not required by the smoke that already passed. Independent review and merge remain ahead; closing #66 stays with Aryan.

## Current October23 — smoke passed, approved merges resume

All three reliability checks pass: sourced English answer without handoff; one private refund card; same-commit Redeploy with clean drain, one post-SIGTERM reply and old instance observed removed after69.197s, stable30s. Live6a815fbf records unchanged6b6d97d; rollback458d4a6f, optional features off. #67 merged94e4f57 after the auto-deploy-off guard. #74 integration7860417 preserves reviewed privacy/#67 code;952/offline8/SQL and independent298-check review pass. Final hosted CI precedes its approved merge; no new-code deploy or memory enablement. #71's guard/shared-page repair and fair concurrent-arm comparison remain. The named timing test is reproduced by withholding loopback progress; its separate test-only fix stays queued. Score47/100.
Expand Down
20 changes: 20 additions & 0 deletions docs/RUNBOOK.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,26 @@ Production uses the existing `DATABASE_URL` for support state as well as knowled

Keep one primary running service. Shared claims and worker slots protect overlap, but the platform still needs an adequate termination grace period. The bot stops taking new work and waits up to 90 seconds for accepted replies/interactions and active Telegram polling. Unstarted work is returned to the queue; a timed-out, started request is an uncertain outcome that must not be automatically replayed.

### Restart drain evidence (#66)

On shutdown the runtime logs one privacy-safe line per accepted case so a bounded log window shows exactly what happened to it. Lines carry only the internal message reference — never customer text, Discord identities or order data:

- `[SupportRuntime] shutdown: finished=<message>` — accepted and settled inside the drain budget. Clean for that case.
- `[SupportRuntime] shutdown: returned unstarted request message=<message>` — accepted but never started; re-queued at once. The replacement picks it up without waiting for a lease to expire.
- `[SupportRuntime] shutdown: left for reclaim message=<message> delivery=not_started` — started but no customer-visible send began. The abandoned sweep will requeue it after the lease lapses; expect one later `requeued abandoned request` line and one answer.
- `[SupportRuntime] shutdown: left for reclaim message=<message> delivery=uncertain` — a customer-visible send may have begun. The request is **not** replayed; after the uncertain grace it is recorded `uncertain`. Inspect the thread before any manual follow-up.
- `[SupportRuntime] shutdown: unstarted return failed message=<message>` — the queue return itself failed; treat the case like `delivery=not_started` and expect the abandoned sweep to recover it.
- `[SupportRuntime] drain completed=true|false telegram_completed=true|false` — the aggregate outcome. `false` means at least one case did not settle in the budget; read the per-case lines above it.

To run the redeploy drain smoke (human-operated; needs the same authorization, guards and rollback target as any release smoke):

1. Use an approved **same-commit redeploy**, never Railway Restart — Restart is an in-container signal that has not documented the teardown grace, and an earlier Restart observation produced a replacement login about 20 s after SIGTERM with no drain line.
2. Arm the log observer first; only then have Aryan send one exact synthetic slow request in the test forum.
3. About 10 s into logged processing, dispatch the redeploy of the exact running commit.
4. In the old instance's logs, find `Received SIGTERM` and then one of the per-case shutdown lines above plus `drain completed=`. No per-case line and no `drain completed=` line means the old process was stopped before it could report — that is platform teardown evidence to raise with Railway, not a clean drain.
5. In the replacement's logs, confirm the login line and that the same message reference produces exactly one customer reply. A `requeued abandoned request` line for it is expected only after a `left for reclaim delivery=not_started` outcome — not after `returned unstarted` (already queued) and never after `delivery=uncertain`.
6. Confirm the old instance is removed within the configured teardown grace (currently 100 s) and keep the thread under a bounded observation interval before claiming there was no duplicate reply.

Before deploying, pass PR tests and relevant answer-quality checks, finish the existing smoke-test prerequisite, and record the current successful deployment as the rollback target. Verify the new deployment, health endpoint, Discord login, staff receipt and actual customer replies. Configuration presence is not a staff-delivery test. Do not change unrelated Railway variables or cloud permissions during rollout.

Remaining staged order is gated #42 → localized closing-card fix → #28 → #29 → #30, with one smoke report and 24 clean hours between releases unless Aryan explicitly says go. Aryan granted that exception for #41, which is live at main `b9eeb16`. Current deployment after the required same-image restart test is `4a8e93d5-771c-486e-91f6-335839f05be4`, created October 12 at 01:02:10 UTC / 06:32:10 IST; Phase 3's rollback target is `6ddd1673-2e45-4cac-9d0d-28c71a9107fd`. REPORT 3 was graded PASS; the reviewer accepted button removal plus tested database notice uniqueness without a live retest. The next deployment needs its own gates and 24 clean hours from this latest restart unless Aryan says go again. Verify the active deployment and interval before dispatch; never use a historical release's hard-coded rollback helper for a later release. No automatic rollout is booked.
Expand Down
29 changes: 26 additions & 3 deletions supportRuntime.js
Original file line number Diff line number Diff line change
Expand Up @@ -468,16 +468,39 @@ class SupportRuntime {
async drain(timeoutMs = 90_000) {
this.stopAccepting();
if (!this.tasks.size) return true;
const watched = new Set(this.jobs.values());
let timer;
try {
const drained = await Promise.race([
Promise.allSettled([...this.tasks]).then(() => true),
new Promise((resolve) => { timer = setTimeout(() => resolve(false), timeoutMs); }),
]);
if (!drained) {
await Promise.all([...this.jobs.values()].filter((job) => !job.started).map((job) => this.store.defer(job)));
const finished = [...watched].filter((job) => !this.jobs.has(job.owner));
if (drained) {
// Every accepted case settled inside the budget; say so per case.
for (const job of finished) this.log(`[SupportRuntime] shutdown: finished=${job.messageId}`);
return true;
}
return drained;
// The budget ran out. Return unstarted work to the recovery queue at once so a
// replacement can pick it up instead of waiting for its lease to expire, and
// name each case we can no longer see so its outcome is read from these lines
// instead of a later abandoned sweep.
const unstarted = [...watched].filter((job) => this.jobs.has(job.owner) && !job.started);
for (const job of unstarted) {
try { await this.store.defer(job); }
catch { this.log(`[SupportRuntime] shutdown: unstarted return failed message=${job.messageId}`); }
}
for (const job of unstarted) {
if (job.lost) this.log(`[SupportRuntime] shutdown: left for reclaim message=${job.messageId} delivery=not_started`);
else this.log(`[SupportRuntime] shutdown: returned unstarted request message=${job.messageId}`);
}
for (const job of watched) {
if (!this.jobs.has(job.owner) || !job.started) continue;
this.log(job.deliveryMarked || job.replySent
? `[SupportRuntime] shutdown: left for reclaim message=${job.messageId} delivery=uncertain`
: `[SupportRuntime] shutdown: left for reclaim message=${job.messageId} delivery=not_started`);
}
return false;
} finally { clearTimeout(timer); }
}
}
Expand Down
88 changes: 88 additions & 0 deletions test/supportRuntime.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,94 @@ test('drain has a bounded timeout and keeps existing ownership while work finish
held.release(); await pending;
});

test('shutdown lets started work finish within the drain budget and logs it finished', async () => {
const logs = [];
const runtime = new SupportRuntime({ log: (line) => logs.push(line) });
const started = barrier(); const proceed = barrier();
const pending = runtime.run(request('finishes'), async () => {
started.release(); await proceed.promise; markCurrentReplySent(); return true;
});
await started.promise;
const draining = runtime.drain(1000);
await new Promise((resolve) => setTimeout(resolve, 20));
proceed.release();
assert.equal(await draining, true);
assert.equal((await pending).status, 'answered');
assert.match(logs.join('\n'), /\[SupportRuntime\] shutdown: finished=finishes/);
assert.doesNotMatch(logs.join('\n'), /returned unstarted request/);
assert.doesNotMatch(logs.join('\n'), /left for reclaim/);
assert.doesNotMatch(logs.join('\n'), /delivery=uncertain/);
});

test('shutdown returns unstarted accepted work to the queue instead of leaving it abandoned', async () => {
let now = 1000;
const store = new MemoryRuntimeStore({ now: () => now }); const logs = [];
const runtime = new SupportRuntime({ store, concurrency: 1, log: (line) => logs.push(line) });
const slotHeld = barrier(); const workStarted = barrier();
const held = runtime.run(request('busy', 'case:a'), async () => { await slotHeld.promise; return true; });
// 'late' passes the durable claim and the per-case gate, then parks on the
// occupied local worker slot with its work never started.
const parked = runtime.run(request('late', 'case:b'), async () => { workStarted.release(); return true; });
await new Promise((resolve) => setTimeout(resolve, 50));
assert.equal((await store.queued(10)).length, 0);
// SIGTERM: no task finishes before the drain timeout.
assert.equal(await runtime.drain(50), false);
assert.equal(store.messages.get('late').state, 'queued', 'unstarted request must be returned to the queue');
assert.match(logs.join('\n'), /\[SupportRuntime\] shutdown: returned unstarted request message=late/);
assert.match(logs.join('\n'), /\[SupportRuntime\] shutdown: left for reclaim message=busy/);
assert.doesNotMatch(logs.join('\n'), /delivery=uncertain/);
slotHeld.release();
await Promise.allSettled([held, parked]);
// The returned request stays recoverable and is answered once by the replacement.
assert.equal((await store.queued(10)).length, 1);
let sends = 0;
const survivor = new SupportRuntime({ store, sweepMs: 0, log: (line) => logs.push(line) });
await survivor.recoverQueued((input) => survivor.run(input, async () => { sends++; return true; }));
assert.equal(sends, 1);
assert.doesNotMatch(logs.join('\n'), /requeued abandoned request message=late/,
'returned work must not surface as an abandoned requeue');
assert.equal(store.messages.get('late').state, 'answered');
});

test('shutdown logs started-unsent and delivery-started work as distinct left-for-reclaim outcomes', async () => {
const store = new MemoryRuntimeStore(); const logs = [];
const runtime = new SupportRuntime({ store, log: (line) => logs.push(line) });
const unsentStarted = barrier(); const delivering = barrier(); const release = barrier();
const a = runtime.run(request('unsent', 'case:a'), async () => { unsentStarted.release(); await release.promise; return true; });
const b = runtime.run(request('delivering', 'case:b'), async () => {
await assertCurrentOwnership(); markCurrentReplySent(); delivering.release(); await release.promise; return true;
});
await Promise.all([unsentStarted.promise, delivering.promise]);
assert.equal(await runtime.drain(50), false);
assert.match(logs.join('\n'), /\[SupportRuntime\] shutdown: left for reclaim message=unsent delivery=not_started/);
assert.match(logs.join('\n'), /\[SupportRuntime\] shutdown: left for reclaim message=delivering delivery=uncertain/);
assert.doesNotMatch(logs.join('\n'), /returned unstarted request/);
release.release();
await Promise.allSettled([a, b]);
});

test('shutdown is not a replay: recovery never resends a request whose delivery began', async () => {
let now = 1000;
const store = new MemoryRuntimeStore({ now: () => now }); const logs = [];
const dead = new SupportRuntime({ store, log: (line) => logs.push(line) });
const deliveryBegan = barrier(); const stop = barrier();
let sends = 0;
dead.run(request('delivering'), async () => {
await assertCurrentOwnership(); sends++; markCurrentReplySent();
deliveryBegan.release(); await stop.promise;
}).catch(() => {});
await deliveryBegan.promise;
now += 120_001;
const survivor = new SupportRuntime({ store, sweepMs: 0, log: (line) => logs.push(line) });
await survivor.recoverQueued((input) => survivor.run(input, async () => { sends++; return true; }));
now += 10 * 60_000;
await survivor.recoverQueued((input) => survivor.run(input, async () => { sends++; return true; }));
assert.equal(sends, 1);
assert.equal(store.messages.get('delivering').state, 'uncertain');
assert.doesNotMatch(logs.join('\n'), /requeued abandoned request message=delivering/);
stop.release();
});

test('lease expiry fences stale replies and does not automatically replay an uncertain started request', async () => {
let now = 1000; const store = new MemoryRuntimeStore({ now: () => now });
const runtime = new SupportRuntime({ store }); const held = barrier(); const started = barrier();
Expand Down