Drain review requests before stopping question agents

Author: mchwang

docs/implementation/read-only-review.md

@@ -100,3 +100,7 @@ The review's polling-efficiency observation is tracked separately in issue #12:
 PR #11 round 7 confirmed cancellation tracking and described outdated snippet retries in its summary. Direct UI reassignment of owned code is already rejected; a persisted assignment change reproduced the stale-reference case, now guarded in both manager and retry UI. The summary also mentioned in-flight composer state: browser tests reproduced lost typing during submission. Action responses now capture current drafts before rendering and clear only the unchanged submitted draft/attachment, preserving edits and drafts on other items.
 
 PR #11 round 8 returned no inline findings and identified one related summary gap: completed answers only warned for an older snapshot or plan revision, not a snippet invalidated by reassignment. Completed answers now show the same historical-context warning for either condition. A browser regression preserves the historical answer while checking the warning after a persisted assignment change.
+
+PR #11 round 9 returned no inline findings and identified shutdown ordering in its summary. Server shutdown now stops accepting connections and drains in-flight HTTP requests before closing the question manager and database. A partial-body request regression proves a question already admitted during shutdown starts its agent and is persisted as interrupted rather than left unanswered.
+
+Current PR validation: 187 unit/integration tests, 31 browser tests, typecheck, and diff checks. Earlier counts above identify the stage when each behavior was added.

test/browser/review.spec.ts

@@ -3,8 +3,10 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { execFileSync } from 'node:child_process';
+import { request as httpRequest } from 'node:http';
 import { createDemo } from '../../scripts/demo.ts';
 import { choiceKeys } from '../../core/approvals.ts';
+import { ReviewService } from '../../runner/review.ts';
 import { startServer } from '../../web/server.ts';
 let root: string, app: Awaited<ReturnType<typeof startServer>>;
 test.beforeEach(async () => { root=mkdtempSync(join(tmpdir(),'codeboost-browser-'));app=await startServer(createDemo(join(root,'demo')),0); });
@@ -258,3 +260,25 @@ test('warns on completed answers when a snippet assignment changes',async({page}
  await expect(page.locator('.agent-answer')).toContainText('Historical answer');
  await expect(page.locator('.agent-answer')).toContainText('Answer refers to earlier code or review context.');
 });
+test('drains an in-flight question request before closing its agent manager',async()=>{
+ const config=app.service.config;await app.close();let calls=0;
+ app=await startServer(config,0,(_prompt,signal)=>{calls++;return new Promise((_,reject)=>signal.addEventListener('abort',()=>reject(signal.reason),{once:true}));});
+ const view=app.service.load(),body=JSON.stringify({action:'note',item:'P1',kind:'question',text:'Question during shutdown',token:view.token});
+ const endpoint=new URL('/api/action',app.url);
+ let response='';
+ const completed=new Promise<void>((resolve,reject)=>{
+  const req=httpRequest(endpoint,{method:'POST',headers:{'x-codeboost-token':app.token,'content-type':'application/json','content-length':Buffer.byteLength(body)}},res=>{
+   res.setEncoding('utf8');res.on('data',chunk=>response+=chunk);res.on('end',resolve);
+  });
+  req.on('error',reject);req.write(body.slice(0,1));
+  setTimeout(()=>req.end(body.slice(1)),50);
+ });
+ await new Promise(resolve=>setTimeout(resolve,10));
+ await Promise.all([app.close(),completed]);
+ const reopened=new ReviewService(config);
+ try {
+  const note=reopened.load().notes.find(note=>note.text==='Question during shutdown');
+  expect(calls).toBe(1);expect(note?.answer?.status).toBe('failed');expect(note?.answer?.error).toMatch(/Server stopped/);
+  expect(JSON.parse(response).notes.some((candidate:{text:string})=>candidate.text==='Question during shutdown')).toBe(true);
+ } finally {reopened.close();app=await startServer(config,0);}
+});

web/server.ts

@@ -56,5 +56,9 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge
   server.requestTimeout = 15000;
   await new Promise<void>((resolve, reject) => { server.once('error', reject); server.listen(port, '127.0.0.1', () => { server.removeListener('error', reject); resolve(); }); }).catch(error => { service.close(); throw error; });
   const address = server.address(); if (!address || typeof address === 'string') throw new Error('Cannot determine local address.');
-  return { server, service, token, url: `http://127.0.0.1:${address.port}/#${token}`, close: async () => { await questions.close(); await new Promise<void>((resolve, reject) => server.close(error => { service.close(); error ? reject(error) : resolve(); })); } };
+  return { server, service, token, url: `http://127.0.0.1:${address.port}/#${token}`, close: async () => {
+    await new Promise<void>((resolve, reject) => server.close(error => error ? reject(error) : resolve()));
+    await questions.close();
+    service.close();
+  } };
 }