From bfe30dfaf98d0be2dfd589da0d07223f4e46ba1a Mon Sep 17 00:00:00 2001 From: mchwang Date: Sat, 26 Sep 2026 04:00:08 -0700 Subject: [PATCH] F1e: planning API for lane G and feedback from review actions Planning endpoints over E3's coordinator and the Store: import, suggestion start/read/cancel/apply, each through Store.userAction; E3's settlement writes use the shutdown capability and the coordinator closes at shutdown. Starting a suggestion is refused until a planning provider is injected. ReviewService.planContext builds the trusted plan context from the base tree. Review actions: change notes, segment accept and assign require an actionId and record their feedback event in the same transaction; a later choice links to the earlier event; choice sources are fixed-size fingerprints. The UI sends an actionId per action. Co-Authored-By: Claude Opus 5.5 --- runner/review.ts | 29 ++++- runner/store.ts | 5 +- test/browser/review.spec.ts | 3 +- test/runner-planning-feedback.test.ts | 173 ++++++++++++++++++++++++++ test/runner-shutdown.test.ts | 8 +- web/public/app.js | 3 +- web/server.ts | 81 +++++++++++- 7 files changed, 290 insertions(+), 12 deletions(-) create mode 100644 test/runner-planning-feedback.test.ts diff --git a/runner/review.ts b/runner/review.ts index 3425aa5..c77d540 100644 --- a/runner/review.ts +++ b/runner/review.ts @@ -4,6 +4,9 @@ import { isDeepStrictEqual } from 'node:util'; import { Store, type ReviewState, type SnippetReference } from './store.ts'; import type { PlanIdentity } from '../core/identity.ts'; import { readHistory } from '../git/history.ts'; +import { execFileSync } from 'node:child_process'; +import { isolatedGitEnvironment } from '../scripts/git-environment.ts'; +import type { BaseEntry, PlanContext } from '../core/plan.ts'; import { linkHistory } from '../core/linking.ts'; import { applyChoices, approvalStates, approveItem, choiceKeys } from '../core/approvals.ts'; import type { GhMergeConfig } from '../github/merge.ts'; @@ -87,7 +90,25 @@ export class ReviewService { const token = createHash('sha256').update(JSON.stringify({ expected, saved, plan, segments })).digest('hex'); return { repository: basename(repository), demo: this.config.demo ?? false, plan, snapshot, expected, token, items, segments, notes, approved: items.filter(item => item.state === 'approved').length }; } - act(input: unknown) { + /** The trusted plan context for import and Apply: base entries from the snapshot's base tree, and the configured path identity. */ + planContext(): PlanContext { + const { identity, repository, pathIdentity } = this.config, plan = this.store.getPlan(identity), snapshot = this.store.getSnapshot(identity); + const pathKey = (path: string) => { + if (!pathIdentity.caseSensitive && /[^\x20-\x7e]/.test(path)) throw new Error('Non-ASCII case-insensitive paths require a filesystem-specific identity adapter.'); + const normalized = pathIdentity.unicodeNormalization === 'NFC' ? path.normalize('NFC') : path; + return pathIdentity.caseSensitive ? normalized : normalized.toLowerCase(); + }; + const listing = execFileSync('git', ['-c', 'core.hooksPath=/dev/null', 'ls-tree', '-rz', snapshot.base], { cwd: repository, env: isolatedGitEnvironment(), encoding: 'utf8', maxBuffer: 64 * 1024 * 1024 }); + const baseEntries: BaseEntry[] = listing.split('\0').filter(Boolean).map(record => { + const split = record.indexOf('\t'), [mode, , oid] = record.slice(0, split).split(' '), path = record.slice(split + 1); + if (mode === '160000') return { path, kind: 'gitlink' }; + if (mode === '120000') return { path, kind: 'symlink', target: execFileSync('git', ['cat-file', 'blob', oid!], { cwd: repository, env: isolatedGitEnvironment(), encoding: 'utf8' }) }; + return { path, kind: 'file' }; + }); + return { identity, issue: plan.issue, baseEntries, pathKey, allowedCommands: [] }; + } + /** With an actionId (inside Store.userAction), feedback-producing actions record their event in the same transaction. */ + act(input: unknown, actionId?: string) { if (!input || typeof input !== 'object') throw new Error('Invalid review command.'); const command = input as Record; const view = this.load(); @@ -105,6 +126,11 @@ export class ReviewService { const item = command.action === 'assign' && typeof command.item === 'string' ? command.item : null; const storedKey = choiceKeys(view.segments, identity)[view.segments.indexOf(segment)]!; this.store.saveReview(identity, view.expected, [], [{ key: storedKey, action: command.action, item }]); + // Choice keys embed segment content and are unbounded; the event's source is a stable fixed-size fingerprint of the key. + const sourceRef = `choice:${createHash('sha256').update(storedKey).digest('hex')}`; + if (actionId) this.store.recordFeedback(identity, actionId, command.action === 'assign' + ? { kind: 'segment-assign', item, sourceRef, supersedeLatest: true } + : { kind: 'segment-accept', sourceRef, supersedeLatest: true }); } else if (command.action === 'note' && typeof command.item === 'string' && typeof command.text === 'string' && (command.kind === 'question' || command.kind === 'change')) { let reference: SnippetReference | undefined; if (command.reference !== undefined) { @@ -121,6 +147,7 @@ export class ReviewService { reference = { key: segment.key, path: segment.operation === '-' ? segment.oldPath ?? segment.path : segment.path, side: segment.operation === '+' ? 'new' : 'old', start, end, text, head: view.snapshot.head, base: view.snapshot.base }; } createdNoteId = this.store.addReviewNote(identity, view.expected, command.item, command.kind, command.text, reference).id; + if (actionId && command.kind === 'change') this.store.recordFeedback(identity, actionId, { kind: 'change-request', item: command.item, text: command.text.trim(), sourceRef: createdNoteId }); } else throw new Error('Unknown review command.'); return { ...this.load(), createdNoteId }; } diff --git a/runner/store.ts b/runner/store.ts index 89279d6..13fae28 100644 --- a/runner/store.ts +++ b/runner/store.ts @@ -760,13 +760,16 @@ export class Store { } } /** Append one feedback event. Call inside userAction so the event and its action share one transaction. */ - recordFeedback(identity: PlanIdentity, actionId: string, event: { kind: Exclude; item?: string | null; text?: string | null; sourceRef: string; supersedes?: string | null }): FeedbackEvent { + recordFeedback(identity: PlanIdentity, actionId: string, event: { kind: Exclude; item?: string | null; text?: string | null; sourceRef: string; supersedes?: string | null; supersedeLatest?: boolean }): FeedbackEvent { assertUuidV4(actionId, 'Action ID'); if (this.#depth === 0) throw new Error('Feedback events are written inside their user action.'); if (!FEEDBACK_KINDS.includes(event.kind) || event.kind === ('task-closed' as FeedbackKind)) throw new GuardRefusal('Invalid feedback kind.'); if (event.text != null && (typeof event.text !== 'string' || event.text.length > 4000)) throw new GuardRefusal('Feedback text is limited to 4000 characters.'); if (typeof event.sourceRef !== 'string' || !event.sourceRef || event.sourceRef.length > 200) throw new GuardRefusal('Invalid feedback source.'); const key = identityKey(identity), plan = this.#current(key); + // A changed segment choice links to the latest earlier event for the same choice key. + if (event.supersedeLatest && event.supersedes == null) + event = { ...event, supersedes: (this.#get("SELECT id FROM feedback_events WHERE plan_key=? AND source_ref=? AND kind IN ('segment-accept','segment-assign') ORDER BY rowid DESC LIMIT 1", key, event.sourceRef)?.id as string | undefined) ?? null }; if (event.supersedes != null && !this.#get('SELECT 1 FROM feedback_events WHERE plan_key=? AND id=? AND source_ref=?', key, event.supersedes, event.sourceRef)) throw new GuardRefusal('A superseded event must belong to the same source.'); const id = randomUUID(), createdAt = new Date().toISOString(); diff --git a/test/browser/review.spec.ts b/test/browser/review.spec.ts index 0ab1032..4fd9213 100644 --- a/test/browser/review.spec.ts +++ b/test/browser/review.spec.ts @@ -1,3 +1,4 @@ +import { randomUUID } from 'node:crypto'; import { test, expect } from '@playwright/test'; import { chmodSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; @@ -138,7 +139,7 @@ test('shows an honest history error and keeps markup in notes as text',async({pa test('can assign a large foreign change without sending its content back in the command',async({request})=>{ const repository=app.service.config.repository;writeFileSync(join(repository,'debug.log'),'x'.repeat(20000)+'\n');execFileSync('git',['-c','core.hooksPath=/dev/null','commit','-am','Large foreign change'],{cwd:repository,stdio:'pipe'}); const base=app.url.split('#')[0]!,headers={'x-codeboost-token':app.token};const view=await(await request.get(base+'api/review',{headers})).json();const segment=view.segments.find((s:{content:string;row:string})=>s.row==='Unplanned'&&s.content.length>19000); - const response=await request.post(base+'api/action',{headers:{...headers,'Content-Type':'application/json'},data:{action:'assign',item:'P1',key:segment.key,token:view.token}}); + const response=await request.post(base+'api/action',{headers:{...headers,'Content-Type':'application/json'},data:{action:'assign',item:'P1',key:segment.key,token:view.token,actionId:randomUUID()}}); expect(response.status()).toBe(200); }); test('shows bounded raster previews and byte sizes for file-change cards',async({page})=>{ diff --git a/test/runner-planning-feedback.test.ts b/test/runner-planning-feedback.test.ts new file mode 100644 index 0000000..f8cfa86 --- /dev/null +++ b/test/runner-planning-feedback.test.ts @@ -0,0 +1,173 @@ +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { createHash, randomUUID } from 'node:crypto'; +import { choiceKeys } from '../core/approvals.ts'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { createDemo } from '../scripts/demo.ts'; +import { startServer, type PlanningDeps } from '../web/server.ts'; +import { Store } from '../runner/store.ts'; +import type { AuthorProvider } from '../core/planning-author.ts'; +import type { EditReply } from '../core/plan.ts'; + +// Integration tests against the real server and git: each review load reads git history, so allow more than vitest's 5 s default. +vi.setConfig({ testTimeout: 20_000 }); +const roots: string[] = [], closers: (() => Promise)[] = []; +afterEach(async () => { + for (const close of closers.splice(0).reverse()) await close(); + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); +}); +type App = Awaited>; +async function serve(options: { planning?: PlanningDeps; questionAgent?: () => Promise } = {}) { + const root = mkdtempSync(join(tmpdir(), 'codeboost-f1e-')); roots.push(root); + const config = createDemo(join(root, 'demo')); + const app = await startServer(config, 0, options.questionAgent ?? (async () => 'answer'), undefined, 2_000, undefined, options.planning); + let closed = false; + const close = async () => { if (!closed) { closed = true; await app.close(); } }; + closers.push(close); + return { app, config, close }; +} +async function api(app: App, method: string, path: string, body?: unknown) { + const response = await fetch(`${new URL(app.url).origin}${path}`, { method, headers: { 'x-codeboost-token': app.token, ...(body ? { 'content-type': 'application/json' } : {}) }, body: body ? JSON.stringify(body) : undefined }); + return { status: response.status, body: await response.json() as Record }; +} +const review = async (app: App) => (await api(app, 'GET', '/api/review')).body; +/** Close the task so lane J's read path opens, then read the events from a fresh Store. */ +async function eventsAfterClose(app: App, config: { database: string; identity: { repositoryId: string; taskId: string; planId: string } }, close: () => Promise) { + const version = (await api(app, 'GET', '/api/runner')).body.stateVersion; + await api(app, 'POST', '/api/runner', { action: 'cancel-task', expectedStateVersion: version, actionId: randomUUID() }); + await close(); + const store = new Store(config.database); + try { return store.feedbackEvents(config.identity); } finally { store.close(); } +} + +describe('feedback from review actions', () => { + it('records a change note and its feedback event together, and replays by action ID', async () => { + const { app, config, close } = await serve(); + const actionId = randomUUID(), note = { action: 'note', kind: 'change', item: 'P1', text: 'Handle the 5xx path', token: (await review(app)).token, actionId }; + expect((await api(app, 'POST', '/api/action', note)).status).toBe(200); + expect((await api(app, 'POST', '/api/action', note)).status).toBe(200); + const saved = (await review(app)).notes.filter((n: { text: string }) => n.text === 'Handle the 5xx path'); + expect(saved).toHaveLength(1); + const events = await eventsAfterClose(app, config, close); + expect(events.map(e => [e.kind, e.item, e.text, e.sourceRef, e.actionId])).toEqual([ + ['change-request', 'P1', 'Handle the 5xx path', saved[0].id, actionId], ['task-closed', null, null, events[1]!.planKey, events[1]!.actionId]]); + }); + it('refuses a feedback-producing action without an action ID and writes nothing', async () => { + const { app } = await serve(); + const response = await api(app, 'POST', '/api/action', { action: 'note', kind: 'change', item: 'P1', text: 'no id', token: (await review(app)).token }); + expect(response).toMatchObject({ status: 400, body: { error: 'actionId is required for this action.' } }); + expect((await review(app)).notes.some((n: { text: string }) => n.text === 'no id')).toBe(false); + }); + it('records a segment choice with a fixed-size source, and links a later choice for the same source', async () => { + const { app, config, close } = await serve(); + const loaded = app.service.load(), index = loaded.segments.findIndex(s => s.row === 'Unplanned'); + const sourceRef = `choice:${createHash('sha256').update(choiceKeys(loaded.segments, config.identity)[index]!).digest('hex')}`; + const accepted = await api(app, 'POST', '/api/action', { action: 'accept', key: loaded.segments[index]!.key, token: loaded.token, actionId: randomUUID() }); + expect(accepted.status).toBe(200); + // The review actions cannot reassign an accepted segment yet, so the later choice goes through the Store directly. + const store = app.service.store, later = randomUUID(); + store.userAction(config.identity, { actionId: later, kind: 'assign', request: {} }, + () => store.recordFeedback(config.identity, later, { kind: 'segment-assign', item: 'P1', sourceRef, supersedeLatest: true })); + const [first, second] = await eventsAfterClose(app, config, close); + expect(first).toMatchObject({ kind: 'segment-accept', sourceRef, supersedes: null }); + expect(sourceRef).toMatch(/^choice:[0-9a-f]{64}$/); + expect(second).toMatchObject({ kind: 'segment-assign', item: 'P1', sourceRef, supersedes: first!.id }); + }); + it('records no feedback for a question, and does not start its agent again on replay', async () => { + let calls = 0; + const { app, config, close } = await serve({ questionAgent: async () => { calls++; return 'answer'; } }); + const question = { action: 'note', kind: 'question', item: 'P1', text: 'Why?', token: (await review(app)).token, actionId: randomUUID() }; + await api(app, 'POST', '/api/action', question); + await api(app, 'POST', '/api/action', question); + for (let i = 0; i < 50 && calls === 0; i++) await new Promise(resolve => setTimeout(resolve, 20)); + expect(calls).toBe(1); + expect((await eventsAfterClose(app, config, close)).map(e => e.kind)).toEqual(['task-closed']); + }); + it('replays a refused review action as the same refusal', async () => { + const { app } = await serve(); + const stale = { action: 'note', kind: 'change', item: 'P1', text: 'late', token: 'stale-token', actionId: randomUUID() }; + const first = await api(app, 'POST', '/api/action', stale); + expect(first.status).toBe(409); + expect(await api(app, 'POST', '/api/action', stale)).toEqual(first); + }); +}); + +describe('planning API for lane G', () => { + const reply = (revision: number): EditReply => ({ schema_version: 1, base_revision: revision, reply: 'Suggestion', edits: [{ op: 'set_field', + item: 'P1', summary: 'Rename', reason: 'Clearer', field: 'title', value: 'Clearer title', file: null, check: null, check_index: null, depends_on: null, new_item: null }] }); + function planning() { + const calls: { signal: AbortSignal; resolve: (reply: string) => void }[] = []; + const provider: AuthorProvider = { invoke: (_request, signal) => new Promise((resolve, reject) => { + calls.push({ signal, resolve }); + signal.addEventListener('abort', () => reject(signal.reason), { once: true }); + }) }; + const deps: PlanningDeps = { provider, describe: () => ({ repo: { name: 'retry-service', baseRef: 'main' }, issue: { number: 3, title: 'Retries', body: '', comments: [] }, approvedLessons: [] }) }; + return { deps, calls }; + } + const until = async (check: () => boolean) => { for (let i = 0; i < 100 && !check(); i++) await new Promise(r => setTimeout(r, 20)); }; + + it('refuses to start suggestions until a planning provider exists', async () => { + const { app } = await serve(); + const view = await review(app); + const response = await api(app, 'POST', '/api/plan/suggestions', { expectedRevision: view.plan.revision, snapshotId: view.snapshot.id, feedback: '', actionId: randomUUID() }); + expect(response).toMatchObject({ status: 409, body: { error: 'Planning agent not available yet.' } }); + }); + it('imports a revision once per action ID and refuses a stale expected revision', async () => { + const { app } = await serve(); + const view = await review(app); + const source = JSON.stringify({ ...view.plan, summary: 'Imported summary' }); + const request = { source, format: 'json', expectedRevision: view.plan.revision, actionId: randomUUID() }; + const first = await api(app, 'POST', '/api/plan/import', request); + expect(first.body.result).toEqual({ revision: view.plan.revision + 1 }); + expect((await api(app, 'POST', '/api/plan/import', request)).body.result).toEqual(first.body.result); + expect((await review(app)).plan.revision).toBe(view.plan.revision + 1); + expect((await api(app, 'POST', '/api/plan/import', { ...request, actionId: randomUUID() })).status).toBe(409); + }); + it('starts, reads, and applies a suggestion exactly once', async () => { + const { deps, calls } = planning(); + const { app } = await serve({ planning: deps }); + const view = await review(app); + const started = await api(app, 'POST', '/api/plan/suggestions', { expectedRevision: view.plan.revision, snapshotId: view.snapshot.id, feedback: 'Clearer titles', actionId: randomUUID() }); + const id = started.body.result.requestId; + expect((await api(app, 'GET', `/api/plan/suggestions/${id}`)).body.state).toBe('pending'); + await until(() => calls.length === 1); + calls[0]!.resolve(JSON.stringify(reply(view.plan.revision))); + let status = (await api(app, 'GET', `/api/plan/suggestions/${id}`)).body; + for (let i = 0; i < 50 && status.state === 'pending'; i++) { await new Promise(r => setTimeout(r, 20)); status = (await api(app, 'GET', `/api/plan/suggestions/${id}`)).body; } + expect(status.state).toBe('ready'); + const apply = { index: 0, actionId: randomUUID() }; + const applied = await api(app, 'POST', `/api/plan/suggestions/${id}/apply`, apply); + expect(applied.body.result).toEqual({ revision: view.plan.revision + 1 }); + expect((await api(app, 'POST', `/api/plan/suggestions/${id}/apply`, apply)).body.result).toEqual(applied.body.result); + expect((await review(app)).plan.revision).toBe(view.plan.revision + 1); + }); + it('cancels a pending suggestion through its handle and a ready one through the Store', async () => { + const { deps, calls } = planning(); + const { app } = await serve({ planning: deps }); + let view = await review(app); + const pending = (await api(app, 'POST', '/api/plan/suggestions', { expectedRevision: view.plan.revision, snapshotId: view.snapshot.id, feedback: '', actionId: randomUUID() })).body.result.requestId; + await until(() => calls.length === 1); + expect((await api(app, 'POST', `/api/plan/suggestions/${pending}/cancel`, { actionId: randomUUID() })).body.result).toEqual({ state: 'cancelling' }); + expect(calls[0]!.signal.aborted).toBe(true); + let state = (await api(app, 'GET', `/api/plan/suggestions/${pending}`)).body.state; + for (let i = 0; i < 50 && state === 'pending'; i++) { await new Promise(r => setTimeout(r, 20)); state = (await api(app, 'GET', `/api/plan/suggestions/${pending}`)).body.state; } + expect(state).toBe('cancelled'); + view = await review(app); + const ready = (await api(app, 'POST', '/api/plan/suggestions', { expectedRevision: view.plan.revision, snapshotId: view.snapshot.id, feedback: '', actionId: randomUUID() })).body.result.requestId; + await until(() => calls.length === 2); + calls[1]!.resolve(JSON.stringify(reply(view.plan.revision))); + for (let i = 0; i < 50 && (await api(app, 'GET', `/api/plan/suggestions/${ready}`)).body.state !== 'ready'; i++) await new Promise(r => setTimeout(r, 20)); + expect((await api(app, 'POST', `/api/plan/suggestions/${ready}/cancel`, { actionId: randomUUID() })).body.result).toEqual({ state: 'cancelled' }); + }); + it('settles a pending suggestion at shutdown instead of leaving it pending', async () => { + const { deps, calls } = planning(); + const { app, config, close } = await serve({ planning: deps }); + const view = await review(app); + const id = (await api(app, 'POST', '/api/plan/suggestions', { expectedRevision: view.plan.revision, snapshotId: view.snapshot.id, feedback: '', actionId: randomUUID() })).body.result.requestId; + await until(() => calls.length === 1); + await close(); + const store = new Store(config.database); + try { expect(store.getSuggestions(config.identity, id).state).not.toBe('pending'); } finally { store.close(); } + }); +}); diff --git a/test/runner-shutdown.test.ts b/test/runner-shutdown.test.ts index ac3db14..d5bf792 100644 --- a/test/runner-shutdown.test.ts +++ b/test/runner-shutdown.test.ts @@ -15,6 +15,8 @@ import type { RunnerDeps } from '../runner/coordinator.ts'; import type { InvocationResult } from '../agents/contract.ts'; import type { MergeGateway, MergeQueueGateway, RemoteMergeState } from '../github/merge.ts'; +// Integration tests against the real server and git: each review load reads git history, so allow more than vitest's 5 s default. +vi.setConfig({ testTimeout: 20_000 }); const roots: string[] = []; const cleanups: (() => Promise | void)[] = []; afterEach(async () => { @@ -123,7 +125,7 @@ describe('server shutdown', () => { it('lets an admitted request finish during the drain (AGENTS.md: drain admitted requests)', async () => { const { app, config, close } = await serve(); const view = (await api(app, 'GET', '/api/review')).body; - const sent = partialPost(app, '/api/action', { action: 'note', kind: 'change', item: 'P1', text: 'admitted note', token: view.token }); + const sent = partialPost(app, '/api/action', { action: 'note', kind: 'change', item: 'P1', text: 'admitted note', token: view.token, actionId: randomUUID() }); await tick(); const closing = close(); await tick(); @@ -136,7 +138,7 @@ describe('server shutdown', () => { it('destroys a request still reading its body after the drain limit, and writes nothing', async () => { const { app, config, close } = await serve(); const view = (await api(app, 'GET', '/api/review')).body; - const sent = partialPost(app, '/api/action', { action: 'note', kind: 'change', item: 'P1', text: 'never finished', token: view.token }); + const sent = partialPost(app, '/api/action', { action: 'note', kind: 'change', item: 'P1', text: 'never finished', token: view.token, actionId: randomUUID() }); sent.response.catch(() => undefined); await tick(); await close(); @@ -148,7 +150,7 @@ describe('server shutdown', () => { const { app, close } = await serve(); const view = (await api(app, 'GET', '/api/review')).body; const closing = close(); - const late = await fetch(`${origin(app)}/api/action`, { method: 'POST', headers: { 'x-codeboost-token': app.token, 'content-type': 'application/json' }, body: JSON.stringify({ action: 'note', kind: 'change', item: 'P1', text: 'late', token: view.token }) }).then(r => r.status, () => 'refused'); + const late = await fetch(`${origin(app)}/api/action`, { method: 'POST', headers: { 'x-codeboost-token': app.token, 'content-type': 'application/json' }, body: JSON.stringify({ action: 'note', kind: 'change', item: 'P1', text: 'late', token: view.token, actionId: randomUUID() }) }).then(r => r.status, () => 'refused'); expect([503, 'refused']).toContain(late); await closing; }); diff --git a/web/public/app.js b/web/public/app.js index 51e6512..29292db 100644 --- a/web/public/app.js +++ b/web/public/app.js @@ -189,7 +189,8 @@ async function act(command) { renderAttachment(); try { rememberDraft(); - const updated = await api("/api/action", { ...command, token: data.token }); + // One action ID per user action: the server replays it exactly and records feedback with it. + const updated = await api("/api/action", { ...command, token: data.token, actionId: crypto.randomUUID() }); rememberDraft(); data = updated; render(); diff --git a/web/server.ts b/web/server.ts index e1c8cb0..eb26624 100644 --- a/web/server.ts +++ b/web/server.ts @@ -8,11 +8,19 @@ import { GhMergeGateway, type MergeGateway } from '../github/merge.ts'; import { MergeCoordinator } from '../runner/merge.ts'; import { RunnerCoordinator, type RunnerDeps } from '../runner/coordinator.ts'; import { BadRequest, GuardRefusal, ShuttingDownError, sameContext } from '../runner/lifecycle.ts'; +import { SuggestionCoordinator, type SuggestionHandle, type SuggestionInput, type SuggestionStore } from '../core/planning-suggestions.ts'; +import type { AuthorProvider } from '../core/planning-author.ts'; +/** Live planning runs only through D (G4 after #51). Until a provider is injected, starting a suggestion is refused. */ +export interface PlanningDeps { + provider: AuthorProvider; + /** Trusted repository, issue and approved-lesson inputs for a suggestion request. */ + describe(): Pick & { repo: { name: string; baseRef: string } }; +} const publicRoot = new URL('./public/', import.meta.url); -export async function startServer(config: ReviewConfig, port = 4318, questionAgent?: QuestionAgent, mergeGateway?: MergeGateway, shutdownDrainMs = 14_500, runnerDeps?: RunnerDeps) { +export async function startServer(config: ReviewConfig, port = 4318, questionAgent?: QuestionAgent, mergeGateway?: MergeGateway, shutdownDrainMs = 14_500, runnerDeps?: RunnerDeps, planning?: PlanningDeps) { if (!Number.isSafeInteger(shutdownDrainMs) || shutdownDrainMs < 1 || shutdownDrainMs > 14_500) throw new Error('Invalid shutdown drain deadline.'); const service = new ReviewService(config), token = randomBytes(32).toString('hex'); - let questions: Questions, merges: MergeCoordinator | null, runner: RunnerCoordinator | null; + let questions: Questions, merges: MergeCoordinator | null, runner: RunnerCoordinator | null, suggestions: SuggestionCoordinator | null; // Only coordinators' settlement and close code receive this; HTTP handlers never do. const capability = service.store.shutdownCapability(); try { @@ -21,6 +29,16 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge merges = !config.demo && (mergeGateway || config.github) ? new MergeCoordinator(service, mergeGateway ?? new GhMergeGateway(config.github!), 14_000, capability) : null; // The runner starts only with an injected D; until #51 lands, runner actions report that it is unavailable. runner = runnerDeps ? new RunnerCoordinator(service.store, runnerDeps, undefined, capability) : null; + // E3's settlement writes (completeSuggestions, settleSuggestion in close()) run with the shutdown capability. + const store = service.store; + const suggestionStore: SuggestionStore = { + getPlan: identity => store.getPlan(identity), getSnapshot: identity => store.getSnapshot(identity), + beginSuggestions: (identity, expected) => store.beginSuggestions(identity, expected), + completeSuggestions: (identity, id, reply) => capability.run(() => store.completeSuggestions(identity, id, reply)), + settleSuggestion: (identity, id, expected, outcome) => capability.run(() => store.settleSuggestion(identity, id, expected, outcome)), + getSuggestions: (identity, id) => store.getSuggestions(identity, id), + }; + suggestions = planning ? new SuggestionCoordinator(suggestionStore, planning.provider) : null; } catch (error) { service.close(); throw error; } const loadReview=()=>{const view=service.load();return {...view,notes:view.notes.map(note=>({...note,answerActive:questions.isRunning(note.id)}))};}; const load=async(signal?:AbortSignal)=>{const view=loadReview();return {...view,merge:merges?await merges.displayStatus(view,signal):{available:false}};}; @@ -58,6 +76,43 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge return { outcome: 'started', attemptId: retry.id }; }).response; }; + /** Handles of suggestion requests started by this process, removed once their outcome settles. */ + const suggestionHandles = new Map(); + const requireAction = (input: Record) => { if (input.actionId === undefined) throw new BadRequest('actionId is required for this action.'); return input.actionId as string; }; + const planningAction = (path: string, input: Record) => { + const actionId = requireAction(input), { actionId: _omit, ...request } = input; + const imported = path === '/api/plan/import', started = path === '/api/plan/suggestions'; + const match = /^\/api\/plan\/suggestions\/([0-9a-f-]{36})\/(cancel|apply)$/.exec(path); + const kind = imported ? 'plan-import' : started ? 'suggestion-start' : `suggestion-${match![2]}`; + return service.store.userAction(identity, { actionId, kind, request }, () => { + if (imported) { + if (typeof input.source !== 'string' || !['json', 'yaml'].includes(input.format as string) || !Number.isSafeInteger(input.expectedRevision)) throw new BadRequest('source, format and expectedRevision are required.'); + return { revision: service.store.importRevision(input.source, input.format as 'json' | 'yaml', service.planContext(), input.expectedRevision as number).revision }; + } + if (started) { + if (!suggestions || !planning) throw new GuardRefusal('Planning agent not available yet.'); + const plan = service.store.getPlan(identity), snapshot = service.store.getSnapshot(identity); + if (input.expectedRevision !== plan.revision || input.snapshotId !== snapshot.id) throw new GuardRefusal('Stale plan revision or snapshot. Reload before asking for suggestions.'); + if (typeof input.feedback !== 'string' || input.feedback.length > 4000) throw new BadRequest('feedback must be text of 4000 characters or fewer.'); + const context = service.planContext(), described = planning.describe(); + const handle = suggestions.start({ context, revision: plan.revision, snapshotId: snapshot.id, issue: described.issue, approvedLessons: described.approvedLessons, feedback: input.feedback, + repo: { ...described.repo, baseSha: snapshot.base, paths: context.baseEntries.map(entry => entry.path) } }); + suggestionHandles.set(handle.id, handle); + void handle.result.finally(() => suggestionHandles.delete(handle.id)); + return { requestId: handle.id }; + } + const id = match![1]!; + if (match![2] === 'cancel') { + const handle = suggestionHandles.get(id), current = service.store.getSuggestions(identity, id); + if (current.state === 'pending' && handle) { handle.cancel('Cancelled by the user.'); return { state: 'cancelling' }; } + // No handle in this process: startup recovery has already stopped any provider (planning runs only through D). + service.store.cancelSuggestions(identity, id, 'Cancelled by the user.'); + return { state: service.store.getSuggestions(identity, id).state }; + } + if (!Number.isSafeInteger(input.index)) throw new BadRequest('index is required.'); + return { revision: service.store.applySuggestion(identity, id, input.index as number, service.planContext()).revision }; + }).response; + }; let stopping = false; const activeRequests=new Set<{abort:AbortController;request:IncomingMessage;readingBody:boolean}>(); const server = createServer(async (req, res) => { @@ -79,7 +134,10 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge if (req.method === 'GET' && path === '/api/merge') { if(!merges)throw new Error('Merging is not configured for this review.');json(200,{queue:await merges.pollQueue()});return; } if (req.method === 'GET' && path === '/api/review') { json(200, await load(requestAbort.signal)); return; } if (req.method === 'GET' && path === '/api/runner') { json(200, runnerView()); return; } - if (req.method !== 'POST' || !['/api/action','/api/settings','/api/runner'].includes(path) || req.headers['content-type'] !== 'application/json') { json(405, { error: 'Unsupported request.' }); return; } + const suggestionRead = /^\/api\/plan\/suggestions\/([0-9a-f-]{36})$/.exec(path); + if (req.method === 'GET' && suggestionRead) { json(200, service.store.getSuggestions(identity, suggestionRead[1]!)); return; } + const planningPath = path === '/api/plan/import' || path === '/api/plan/suggestions' || /^\/api\/plan\/suggestions\/[0-9a-f-]{36}\/(cancel|apply)$/.test(path); + if (req.method !== 'POST' || !(['/api/action','/api/settings','/api/runner'].includes(path) || planningPath) || req.headers['content-type'] !== 'application/json') { json(405, { error: 'Unsupported request.' }); return; } const chunks: Buffer[] = []; let size = 0; activeRequest.readingBody=true; try { for await (const chunk of req) { size += chunk.length; if (size > 16384) { json(413, { error: 'Request too large.' }); return; } chunks.push(chunk); } } @@ -89,6 +147,7 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge // Admitted requests drain normally (AGENTS.md); only the irreversible merge boundary rechecks the flag. if (stopping && input.action === 'merge') { json(503, { error: 'The review server is shutting down.' }); return; } if(path==='/api/runner') { json(200, { result: runnerAction(input), runner: runnerView() }); return; } + if(planningPath) { json(200, { result: planningAction(path, input) }); return; } if(path==='/api/settings') {service.store.setQuestionProvider(input.questionProvider);json(200,{questionProvider:service.store.questionProvider()});return;} if(input.action==='retry-question') { const view=service.load();if(input.token!==view.token)throw new Error('Stale review state. Refresh and retry.'); @@ -101,8 +160,19 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge catch { json(200,{mergeResult:merged.result,mergeQueue:null,mergeRefreshRequired:true}); } return; } - const view=service.act(input); - if(view.createdNoteId && input.kind==='question') { + // Feedback-producing actions need an actionId: the action and its feedback event share one transaction. + const feedback = input.action==='accept' || input.action==='assign' || (input.action==='note' && input.kind==='change'); + if (feedback) requireAction(input); + let view: ReturnType, replayed = false; + if (input.actionId !== undefined) { + const { actionId, ...request } = input; + const outcome = service.store.userAction(identity, { actionId, kind: `review-${String(input.action).replace(/[^a-z-]/g, '')}`, request }, () => { + const acted = service.act(input, actionId); return { createdNoteId: acted.createdNoteId ?? null }; + }); + replayed = outcome.replayed; + view = { ...service.load(), createdNoteId: outcome.response.createdNoteId ?? undefined }; + } else view=service.act(input); + if(view.createdNoteId && input.kind==='question' && !replayed) { try {questions.start(view.createdNoteId,view);} catch(error) { // The saved question remains visible and retryable when capacity is reached. } @@ -156,6 +226,7 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge await runner?.close(); await closing; await questions.close(); + await suggestions?.close(); service.close(); } }; }