From 37255f511940f1e03bee5f2670619424701ee9a2 Mon Sep 17 00:00:00 2001 From: mchwang Date: Thu, 8 Oct 2026 10:03:33 -0700 Subject: [PATCH 01/10] Add G2 planning authoring controls --- docs/implementation/planning-screen.md | 16 ++- test/browser/plans-authoring.spec.ts | 183 +++++++++++++++++++++++++ web/public/app.js | 182 ++++++++++++++++++++++++ web/public/index.html | 34 +++-- web/public/style.css | 80 ++++++++++- 5 files changed, 482 insertions(+), 13 deletions(-) create mode 100644 test/browser/plans-authoring.spec.ts diff --git a/docs/implementation/planning-screen.md b/docs/implementation/planning-screen.md index a88f066f..ac1bf5ab 100644 --- a/docs/implementation/planning-screen.md +++ b/docs/implementation/planning-screen.md @@ -1,9 +1,11 @@ # Planning screen -**Lane and step:** lane G, step G1. Tracking issue: #145. +**Lane and steps:** lane G, G1 and G2. Tracking issue: #145. G1 adds a Plans screen over the planning contracts already owned by lanes E and F. It does not add another plan writer: the browser imports through `POST /api/plan/import`, then reloads `GET /api/review` so the current Store revision remains the only displayed authority. +G2 adds browser controls for the existing draft and suggestion request lifecycles. The screen can ask for a complete next-revision draft or independent edit cards, poll the durable request, render the result and dismiss it. It still cannot apply either result; Apply remains G3. + ## Display contract The screen renders the complete current plan: issue, summary, revision, open questions, ordered items, dependencies, files (including both sides of a rename) and acceptance checks. Navigation uses `?view=plans`, announces the active destination with `aria-current`, and preserves the existing Review composer DOM while the Plans screen is open. An authoritative Review refresh reconciles a settled Plans revision or load-failure status with the response it applies, but only while no newer plan operation owns that status. A failed Review refresh still clears unavailable plan data, but likewise replaces the Plans status only while it retains ownership. @@ -18,4 +20,14 @@ The Store validates schema, issue identity, revision, paths, dependencies and co `test/browser/plans.spec.ts` covers JSON and YAML imports, full plan and rename rendering, navigation/reload, unsent Review draft preservation, duplicate activation and focus, a newer file selection surviving an older response, exact ambiguous retry before and after a refresh observes a lost-response commit, retry preservation across another file's refusal, committed-import/reload-failure reporting, post-import Review writes while a superseded read remains unsettled, immediate stale Plans UI release after an irreversible merge, both orderings of Review/Plans refreshes against terminal merge and question observations, terminal full-response authority over matching terminal and active polls, submitting-to-queued progress in both response orders, cancellation settlement, both response orderings between review actions and merge polls, queue-poll resumption after a rejected import, and an issue-mismatch refusal that leaves the Store at the original revision. -G2 adds authoring and suggestion cards. G3 adds revision-bound Apply and request-time draft/attachment preservation. G4 completes real-provider/store integration and the remaining T18 browser/adapter gate. +## Authoring and suggestions + +The guidance field is shared by the two explicit request types. **Draft next revision** starts `/api/plan/drafts`; **Suggest edits** starts `/api/plan/suggestions`. Each request is bound to the displayed revision and repository snapshot and carries a UUID action ID. If the start response is ambiguous, the matching control retries that exact body and action ID; the other request type stays unavailable until the outcome is known. + +After start, the screen polls only the durable request resource with bounded backoff. A lifecycle transition resets the interval. Poll updates replace only the request status and result: they never write the guidance field. The active control uses `aria-disabled` with an in-flight guard, so keyboard focus stays in place and repeat activation cannot start a second invocation. Dismiss uses the request's cancel endpoint and its own stable action ID. + +Suggestion replies render as plain text followed by one line-separated row per typed edit. Drafts render the proposed revision summary and ordered items. The browser escapes every agent-provided field. A result remains visible as history but is marked stale whenever its captured revision or snapshot no longer matches the current review response. A delayed ready poll therefore cannot present an old result as current after an import or refresh advances the plan. + +`test/browser/plans-authoring.spec.ts` covers duplicate activation and focus, request-time guidance preservation, suggestion rows, whole-draft preview without mutation, dismissal without a revision change, and the interleaving where an import advances the plan before an older ready poll returns. + +G3 adds revision-bound Apply and request-time draft/attachment preservation. G4 completes real-provider/store integration and the remaining T18 browser/adapter gate. diff --git a/test/browser/plans-authoring.spec.ts b/test/browser/plans-authoring.spec.ts new file mode 100644 index 00000000..6642da73 --- /dev/null +++ b/test/browser/plans-authoring.spec.ts @@ -0,0 +1,183 @@ +import { test, expect, type Page } from '@playwright/test'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { createDemo } from '../../scripts/demo.ts'; +import { startServer, type PlanningDeps } from '../../web/server.ts'; +import type { AuthorProvider, AuthorRequest } from '../../core/planning-author.ts'; +import type { EditReply, Plan } from '../../core/plan.ts'; + +let root: string, app: Awaited>; +type Invocation = { request: AuthorRequest; signal: AbortSignal; resolve: (source: string) => void }; + +function planning() { + const invocations: Invocation[] = []; + const provider: AuthorProvider = { invoke: (request, signal) => new Promise((resolve, reject) => { + invocations.push({ request, 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: [], + validate: () => undefined, + }), + }; + return { deps, invocations }; +} + +test.beforeEach(async () => { + root = mkdtempSync(join(tmpdir(), 'codeboost-plans-authoring-browser-')); +}); +test.afterEach(async () => { + await app?.close(); + rmSync(root, { recursive: true, force: true }); +}); + +async function openPlans(page: Page, deps: PlanningDeps) { + app = await startServer(createDemo(join(root, 'demo')), 0, undefined, undefined, undefined, undefined, undefined, deps); + await page.goto(app.url); + await page.getByRole('link', { name: 'Plans', exact: true }).click(); +} + +function suggestions(revision = 1): EditReply { + return { + schema_version: 1, + base_revision: revision, + reply: 'The retry title can state the operator-visible outcome more directly.', + edits: [{ + op: 'set_field', item: 'P1', summary: 'Name the retry ceiling', reason: 'Operators need to know that retries stay bounded.', + field: 'title', value: 'Expose the bounded retry ceiling', file: null, check: null, check_index: null, depends_on: null, new_item: null, + }], + }; +} + +function draft(current: Plan): Plan { + return { + ...structuredClone(current), + revision: current.revision + 1, + summary: 'Make retry limits visible to operators', + items: current.items.map((item, index) => index === 0 ? { ...item, title: 'Expose the bounded retry ceiling' } : item), + }; +} + +test('renders suggestion cards and preserves guidance edited after submission', async ({ page }) => { + const { deps, invocations } = planning(); + await openPlans(page, deps); + const guidance = page.getByLabel('Guidance'); + await guidance.fill('Make the plan clearer.'); + const suggest = page.locator('#plan-suggest'); + await suggest.focus(); + await suggest.evaluate((button: HTMLButtonElement) => { button.click(); button.click(); }); + + await expect.poll(() => invocations.length).toBe(1); + await expect(suggest).toBeFocused(); + await expect(suggest).toHaveAttribute('aria-disabled', 'true'); + await guidance.fill('Keep these newer notes while the request runs.'); + invocations[0]!.resolve(JSON.stringify(suggestions())); + + await expect(page.getByRole('status').filter({ hasText: 'Suggestions ready for r1.' })).toBeVisible(); + await expect(page.getByRole('article', { name: 'Name the retry ceiling' })).toContainText('Set P1.title to Expose the bounded retry ceiling'); + await expect(guidance).toHaveValue('Keep these newer notes while the request runs.'); + await page.screenshot({ path: 'test-results/plans-authoring-desktop.png', fullPage: true }); + expect(invocations[0]!.request).toMatchObject({ mode: 'suggest', revision: 1 }); + expect(invocations[0]!.request.prompt).toContain('Make the plan clearer.'); +}); + +test('renders a whole next-revision draft without changing the current plan', async ({ page }) => { + const { deps, invocations } = planning(); + await openPlans(page, deps); + const current = app.service.store.getPlan(app.service.config.identity); + await page.getByLabel('Guidance').fill('Rewrite the plan around operator visibility.'); + await page.getByRole('button', { name: 'Draft next revision' }).click(); + await expect.poll(() => invocations.length).toBe(1); + invocations[0]!.resolve(JSON.stringify(draft(current))); + + await expect(page.getByRole('status').filter({ hasText: 'Draft ready for r1.' })).toBeVisible(); + await expect(page.getByRole('region', { name: 'Draft Make retry limits visible to operators' })).toContainText('P1 Expose the bounded retry ceiling'); + await expect(page.locator('#plans-revision')).toHaveText('r1'); + expect(app.service.store.getPlan(app.service.config.identity).revision).toBe(1); +}); + +test('retries an ambiguous suggestion start with the exact request', async ({ page }) => { + const { deps, invocations } = planning(); + await openPlans(page, deps); + const sent: unknown[] = []; + await page.route('**/api/plan/suggestions', async route => { + sent.push(route.request().postDataJSON()); + if (sent.length === 1) await route.abort('failed'); + else await route.continue(); + }); + const guidance = page.getByLabel('Guidance'); + await guidance.fill('Use the submitted guidance.'); + await page.getByRole('button', { name: 'Suggest edits' }).click(); + await expect(page.getByRole('status').filter({ hasText: 'request outcome is unknown' })).toBeVisible(); + await guidance.fill('Do not replace these newer notes.'); + await page.getByRole('button', { name: 'Retry suggestion request' }).click(); + await expect.poll(() => invocations.length).toBe(1); + expect(sent).toHaveLength(2); + expect(sent[1]).toEqual(sent[0]); + expect(invocations[0]!.request.prompt).toContain('Use the submitted guidance.'); + invocations[0]!.resolve(JSON.stringify(suggestions())); + await expect(page.getByRole('status').filter({ hasText: 'Suggestions ready for r1.' })).toBeVisible(); + await expect(guidance).toHaveValue('Do not replace these newer notes.'); +}); + +test('dismisses ready suggestions without changing the plan', async ({ page }) => { + const { deps, invocations } = planning(); + await openPlans(page, deps); + await page.getByRole('button', { name: 'Suggest edits' }).click(); + await expect.poll(() => invocations.length).toBe(1); + invocations[0]!.resolve(JSON.stringify(suggestions())); + const dismiss = page.locator('#plan-author-dismiss'); + await expect(dismiss).toBeVisible(); + let release!: () => void, arrived!: () => void; + const held = new Promise(resolve => { release = resolve; }); + const captured = new Promise(resolve => { arrived = resolve; }); + await page.route('**/api/plan/suggestions/*/cancel', async route => { arrived(); await held; await route.continue(); }); + await dismiss.focus(); + await dismiss.click(); + await captured; + await expect(dismiss).toBeFocused(); + await expect(dismiss).toHaveAttribute('aria-disabled', 'true'); + release(); + await expect(page.getByRole('status').filter({ hasText: 'Suggestions dismissed.' })).toBeVisible(); + expect(app.service.store.getPlan(app.service.config.identity).revision).toBe(1); +}); + +test('marks a delayed suggestion poll stale after an import advances the plan', async ({ page }) => { + const { deps, invocations } = planning(); + await openPlans(page, deps); + const current = app.service.store.getPlan(app.service.config.identity); + let release!: () => void, arrived!: () => void; + const held = new Promise(resolve => { release = resolve; }); + const captured = new Promise(resolve => { arrived = resolve; }); + let heldReady = false; + await page.route('**/api/plan/suggestions/*', async route => { + const response = await route.fetch(); + const body = await response.json(); + if (!heldReady && body.state === 'ready') { + heldReady = true; + arrived(); + await held; + } + await route.fulfill({ response, body: JSON.stringify(body) }); + }); + await page.getByRole('button', { name: 'Suggest edits' }).click(); + await expect.poll(() => invocations.length).toBe(1); + invocations[0]!.resolve(JSON.stringify(suggestions())); + await captured; + + await page.getByLabel('Plan file').setInputFiles({ + name: 'next-plan.json', mimeType: 'application/json', buffer: Buffer.from(JSON.stringify(draft(current))), + }); + await page.getByRole('button', { name: 'Import next revision' }).click(); + await expect(page.locator('#plans-revision')).toHaveText('r2'); + release(); + + await expect(page.getByRole('status').filter({ hasText: 'Stale suggestions · generated for r1' })).toBeVisible(); + await expect(page.getByRole('article', { name: 'Name the retry ceiling' })).toBeVisible(); + expect(app.service.store.getPlan(app.service.config.identity).revision).toBe(2); +}); diff --git a/web/public/app.js b/web/public/app.js index 60f6c423..9a8a1978 100644 --- a/web/public/app.js +++ b/web/public/app.js @@ -905,6 +905,10 @@ let planImportPending = false, planRefreshPending = false, planOperationGeneration = 0, planStatusGeneration = 0; +let planAuthorGeneration = 0, + planAuthorRequest = null; +const planAuthorPollInitial = 500, + planAuthorPollMaximum = 5000; function claimPlanStatus(className, text) { const owner = ++planStatusGeneration; $("plans-status").className = className; @@ -924,12 +928,85 @@ function supersedePlanRefresh() { $("plans-refresh").textContent = "Refresh plan"; claimPlanStatus("neutral", ""); } +function planAuthorBusy() { + return ["starting", "pending", "cancelling"].includes(planAuthorRequest?.state); +} +function planAuthorStale(request = planAuthorRequest) { + return !!request && (!data?.plan || + request.revision !== data.plan.revision || request.snapshotId !== data.snapshot?.id); +} +function describePlanEdit(edit) { + switch (edit.op) { + case "add_item": return `Add ${edit.item}`; + case "remove_item": return `Remove ${edit.item}`; + case "set_field": return `Set ${edit.item}.${edit.field} to ${edit.value}`; + case "add_file": return `Add ${edit.file?.path} to ${edit.item}`; + case "update_file": return `Update ${edit.file?.path} in ${edit.item}`; + case "remove_file": return `Remove ${edit.value} from ${edit.item}`; + case "add_check": return `Add ${edit.check?.type} check to ${edit.item}`; + case "remove_check": return `Remove check ${Number(edit.check_index) + 1} from ${edit.item}`; + case "set_depends": return `Set ${edit.item} dependencies to ${(edit.depends_on || []).join(", ") || "none"}`; + default: return "Change the plan"; + } +} +function renderPlanAuthor() { + const restoreDismissFocus = document.activeElement?.id === "plan-author-dismiss"; + const request = planAuthorRequest, unavailable = !data?.plan; + const busyAuthor = planAuthorBusy(), uncertain = request?.state === "uncertain"; + for (const [id, mode, label] of [["plan-draft", "draft", "Draft next revision"], ["plan-suggest", "suggest", "Suggest edits"]]) { + const button = $(id); + const blocked = unavailable || busyAuthor || (uncertain && request.mode !== mode); + if (blocked) button.setAttribute("aria-disabled", "true"); + else button.removeAttribute("aria-disabled"); + button.textContent = request?.state === "starting" && request.mode === mode ? "Starting…" + : request?.state === "pending" && request.mode === mode ? "Working…" + : request?.state === "cancelling" && request.mode === mode ? "Cancelling…" + : uncertain && request.mode === mode ? `Retry ${mode === "draft" ? "draft" : "suggestion"} request` + : label; + } + if (!request) { + $("plan-author-status").className = "neutral"; + $("plan-author-status").textContent = unavailable ? "Load a plan before asking the agent." : ""; + $("plan-author-result").innerHTML = ""; + return; + } + const noun = request.mode === "draft" ? "draft" : "suggestions", stale = planAuthorStale(request); + let tone = "neutral", message = ""; + if (stale) { tone = "warn"; message = `! Stale ${noun} · generated for r${request.revision}`; } + else if (request.state === "starting") message = `Starting ${noun}…`; + else if (request.state === "pending") message = `Agent is preparing ${noun} for r${request.revision}…`; + else if (request.state === "cancelling") message = `Cancelling ${noun}…`; + else if (request.state === "uncertain") { tone = "warn"; message = `! The ${noun} request outcome is unknown. Retry the exact request.`; } + else if (request.state === "ready") { tone = "good"; message = `✓ ${request.mode === "draft" ? "Draft" : "Suggestions"} ready for r${request.revision}.`; } + else if (request.state === "failed") { tone = "bad"; message = `✕ ${request.mode === "draft" ? "Draft" : "Suggestion"} failed. ${request.reason || "Try again."}`; } + else if (request.state === "invalidated") { tone = "warn"; message = `! ${request.mode === "draft" ? "Draft" : "Suggestions"} became stale. ${request.reason || "Reload and ask again."}`; } + else if (request.state === "cancelled") message = `– ${request.mode === "draft" ? "Draft" : "Suggestions"} dismissed. ${request.reason || ""}`.trim(); + if (request.observationError) message += ` Status check failed: ${request.observationError}`; + $("plan-author-status").className = tone; + $("plan-author-status").textContent = message; + let result = ""; + if (request.mode === "suggest" && request.reply) { + result = `
Suggested edits r${esc(request.revision)}
+ ${request.reply.reply ? `

${esc(request.reply.reply)}

` : ""} + ${request.reply.edits.map((edit) => `
${esc(edit.item)}

${esc(edit.summary)}

${esc(edit.reason)}

${esc(describePlanEdit(edit))}

`).join("") || '

No edits were suggested.

'}`; + } else if (request.mode === "draft" && request.plan) { + result = `
Draft revision r${esc(request.plan.revision)}
+

${esc(request.plan.summary)}

+
    ${request.plan.items.map((item) => `
  • ${esc(item.id)} ${esc(item.title)}
    • `).join("")}
`; + } + if (result && request.requestId && !["cancelled", "consumed"].includes(request.state)) + result += `
`; + $("plan-author-result").innerHTML = result; + $("plan-author-dismiss")?.addEventListener("click", dismissPlanAuthor); + if (restoreDismissFocus) $("plan-author-dismiss")?.focus(); +} function renderPlans() { if (!data?.plan) { $("plans-summary").textContent = "Plan unavailable"; $("plans-revision").textContent = ""; $("plans-items").innerHTML = ""; $("plans-questions").innerHTML = ""; + renderPlanAuthor(); return; } const plan = data.plan; @@ -949,6 +1026,7 @@ function renderPlans() {

Acceptance

    ${item.acceptance.map((check) => `
  • ${esc(check.type)}${esc(check.text)}
    • `).join("")}
`).join(""); + renderPlanAuthor(); } function planFileFormat(name) { return /\.ya?ml$/i.test(name) ? "yaml" : /\.json$/i.test(name) ? "json" : null; @@ -1048,6 +1126,108 @@ async function importPlan(event) { $("plan-import").textContent = "Import next revision"; } } +function schedulePlanAuthorPoll(generation, delay = planAuthorRequest?.pollDelay ?? planAuthorPollInitial) { + const request = planAuthorRequest; + if (!request || request.generation !== generation || !request.requestId) return; + if (request.pollTimer) clearTimeout(request.pollTimer); + request.pollTimer = setTimeout(() => pollPlanAuthor(generation), delay); +} +async function pollPlanAuthor(generation) { + const request = planAuthorRequest; + if (!request || request.generation !== generation || !request.requestId) return; + request.pollTimer = null; + try { + const status = await api(`/api/plan/${request.mode === "draft" ? "drafts" : "suggestions"}/${request.requestId}`); + if (planAuthorRequest !== request || request.generation !== generation) return; + const changed = request.state !== status.state; + request.state = status.state; + request.revision = status.revision; + request.snapshotId = status.snapshotId; + request.reason = status.reason; + request.observationError = null; + if (request.mode === "draft") request.plan = status.plan; + else request.reply = status.reply; + request.pollDelay = changed ? planAuthorPollInitial : Math.min(request.pollDelay * 2, planAuthorPollMaximum); + renderPlanAuthor(); + if (request.state === "pending") schedulePlanAuthorPoll(generation); + } catch (error) { + if (planAuthorRequest !== request || request.generation !== generation) return; + request.observationError = error.message; + request.pollDelay = Math.min(request.pollDelay * 2, planAuthorPollMaximum); + renderPlanAuthor(); + schedulePlanAuthorPoll(generation); + } +} +async function startPlanAuthor(mode) { + if (!data?.plan || planAuthorBusy()) return; + if (planImportPending || planRefreshPending || busy) { + $("plan-author-status").className = "warn"; + $("plan-author-status").textContent = "! Wait for the current plan or review action before asking the agent."; + return; + } + if (planAuthorRequest?.state === "uncertain" && planAuthorRequest.mode !== mode) return; + const retry = planAuthorRequest?.state === "uncertain" && planAuthorRequest.mode === mode; + const requestBody = retry ? planAuthorRequest.request : { + expectedRevision: data.plan.revision, + snapshotId: data.snapshot.id, + feedback: $("plan-feedback").value, + actionId: crypto.randomUUID(), + }; + const request = { + generation: ++planAuthorGeneration, + mode, + state: "starting", + revision: requestBody.expectedRevision, + snapshotId: requestBody.snapshotId, + request: requestBody, + requestId: null, + reason: null, + reply: null, + plan: null, + observationError: null, + pollDelay: planAuthorPollInitial, + pollTimer: null, + cancelActionId: null, + }; + planAuthorRequest = request; + renderPlanAuthor(); + try { + const response = await api(`/api/plan/${mode === "draft" ? "drafts" : "suggestions"}`, requestBody); + if (planAuthorRequest !== request) return; + const requestId = response.result?.requestId; + if (typeof requestId !== "string") throw new Error("The planning request started without an ID."); + request.requestId = requestId; + request.state = "pending"; + renderPlanAuthor(); + schedulePlanAuthorPoll(request.generation, 0); + } catch (error) { + if (planAuthorRequest !== request) return; + const ambiguous = !Number.isSafeInteger(error?.status) || error.status === 503 || error.outcomeUnknown === true; + request.state = ambiguous ? "uncertain" : "failed"; + request.reason = error.message; + renderPlanAuthor(); + } +} +async function dismissPlanAuthor() { + const request = planAuthorRequest; + if (!request?.requestId || planAuthorBusy()) return; + request.cancelActionId ??= crypto.randomUUID(); + const previousState = request.state; + request.state = "cancelling"; + renderPlanAuthor(); + try { + await api(`/api/plan/${request.mode === "draft" ? "drafts" : "suggestions"}/${request.requestId}/cancel`, { actionId: request.cancelActionId }); + if (planAuthorRequest !== request) return; + request.observationError = null; + schedulePlanAuthorPoll(request.generation, 0); + } catch (error) { + if (planAuthorRequest !== request) return; + request.state = previousState; + request.observationError = error.message; + renderPlanAuthor(); + schedulePlanAuthorPoll(request.generation); + } +} function showView(next) { view = next; $("review-view").hidden = next !== "review"; @@ -1284,6 +1464,8 @@ $("plans-refresh").onclick = async () => { }; $("plan-file").onchange = renderPlanFile; $("plan-import-form").onsubmit = importPlan; +$("plan-draft").onclick = () => startPlanAuthor("draft"); +$("plan-suggest").onclick = () => startPlanAuthor("suggest"); $("issues-refresh").onclick = () => loadIssues(); $("issues-list").onclick = (event) => { const button = event.target.closest("button.issue-trust"); diff --git a/web/public/index.html b/web/public/index.html index 6deb37cd..9e366630 100644 --- a/web/public/index.html +++ b/web/public/index.html @@ -79,15 +79,31 @@

Loading plan…

-