From ab3182fd08a29d79bcef5c8fbf819e7190bffaaa Mon Sep 17 00:00:00 2001 From: mingchuno Date: Thu, 22 Oct 2026 09:26:02 +0100 Subject: [PATCH] feat(sdlc): approve with short issue comments and reopen the web monitor - provider approval accepts `/approve GATE CODE`, where CODE names the gate's contract content printed in the published issue section; it counts only while the issue shows the current contract and after the Work Item last changed (ADR 0013); the long form still works - inspections expose approvalComment, shown by start-work, the TUI and the web monitor - run keeps the web monitor link in its control record, falls back to a free port when the default is taken, and `monitor --web [WORK_ID]` reopens it - `#work=ID` links open the web monitor at a Work Item; work commands and doctor return token-free monitorUrl links Co-Authored-By: Claude Opus 5.5 --- docs/adr/0013-short-approval-comments.md | 23 +++++++ docs/adr/README.md | 1 + docs/engineering/sdlc.md | 21 +++--- docs/operations.md | 15 ++-- packages/cli/src/cli-runner-control.ts | 29 +++++++- packages/cli/src/cli.ts | 45 ++++++++++-- packages/cli/src/doctor.ts | 9 ++- packages/cli/src/tui/work-items.tsx | 3 + packages/cli/src/web/lifecycle.ts | 21 ++++-- packages/cli/src/web/server.ts | 7 +- packages/cli/src/web/worker.ts | 28 +++++++- packages/cli/src/work-commands.ts | 17 ++++- packages/cli/web/app.tsx | 15 +++- packages/cli/web/work-items.tsx | 27 ++++++-- packages/core/src/adapters/hosting.ts | 2 + packages/core/src/domain.ts | 2 + packages/core/src/work/approvals.ts | 20 +++++- packages/core/src/work/policy.ts | 18 +++++ packages/core/src/work/publication.ts | 36 +++++++++- packages/core/src/work/store.ts | 16 ++++- skills/start-work/references/delegated.md | 11 ++- skills/start-work/references/work-api.md | 3 +- tests/web.test.ts | 57 ++++++++++++++- tests/work-console.test.ts | 34 +++++++++ tests/work-publication.test.ts | 84 ++++++++++++++++++++++- 25 files changed, 494 insertions(+), 50 deletions(-) create mode 100644 docs/adr/0013-short-approval-comments.md diff --git a/docs/adr/0013-short-approval-comments.md b/docs/adr/0013-short-approval-comments.md new file mode 100644 index 0000000..5e97548 --- /dev/null +++ b/docs/adr/0013-short-approval-comments.md @@ -0,0 +1,23 @@ +# Approve on the issue with a short contract code + +Provider approval required an allowed human to paste +`agent-workflows approve WORK_ID GATE REVISION`, with a UUID and a 64-character +Work Item revision that appeared nowhere on the issue. The revision cannot be +printed there: it covers the issue body, so printing it would change it +(ADR 0011). + +A short comment `/approve GATE CODE` also approves. `CODE` is the first twelve +hex characters of the gate's contract digest without the assessed source +revision, so the published contract can print the exact comment. The Work Item +is the one whose contract the issue carries. Synchronization accepts a short +comment only when the issue still shows the current contract, the issue +revision is the one the Work Item observed, and the comment was posted after +the Work Item last changed. The last rule keeps ADR 0009's guarantee that a +source edit invalidates earlier authorization, even when reassessment leaves +the contract text unchanged. + +Approval still binds what the approver can read. The code is a name for the +contract, not a secret; authority comes from the host-authenticated allowlisted +author. Comparing host comment time with the local clock can ignore a comment +posted within the skew after publication; reposting fixes it. The full form +remains valid. diff --git a/docs/adr/README.md b/docs/adr/README.md index d233d19..2ac6be0 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -16,6 +16,7 @@ documentation. - [0010: Run Track A inline and delegate only gated work](0010-inline-fast-path.md) - [0011: Publish clarified contracts to the issue](0011-publish-contracts-to-issues.md) - [0012: Onboard through start-work and existing sign-ins](0012-onboard-through-start-work.md) +- [0013: Approve on the issue with a short contract code](0013-short-approval-comments.md) For observable behavior, see the [public SDK API](../api.md), [configuration reference](../configuration.md), and [operator guide](../operations.md). diff --git a/docs/engineering/sdlc.md b/docs/engineering/sdlc.md index 0c61c9d..7f967c8 100644 --- a/docs/engineering/sdlc.md +++ b/docs/engineering/sdlc.md @@ -162,18 +162,23 @@ recorded safety checks permit them. ## Keep enterprise approval independent The enterprise example requires GitLab approval by the configured allowed human. -Local `work approve` rejects that policy. After inspecting the current contract, -the human posts this exact plain comment on the source issue. +Local `work approve` rejects that policy. The published contract ends with the +exact comment for each gate, and `start-work`, `work inspect` and the monitors +show it for the gate awaiting approval. After reading the contract on the issue, +the approver posts it as a plain comment: ```text -agent-workflows approve WORK_ID scope CURRENT_REVISION +/approve scope 3f9a2c1d4e5f ``` -For the design gate, the human substitutes `design` and the complete revision -they inspected. The runner reads source-host comments and accepts only configured -human approvers at the current revision. It ignores bots, outsiders, and stale -comments. The intake agent never posts approval comments. Configuration loading -rejects a `github` or `gitlab` approval source without a nonempty `approvers` list. +The code is a digest of the gate's contract content, so it names the contract +printed on the issue. A short comment counts only while the issue shows the Work +Item's current contract, and only when posted after the Work Item last changed, +so a comment from before a source edit or republication never approves the +reassessed work. That comparison uses the host's comment time and the local +clock; if a valid comment is ignored because of clock skew, post it again. The +long form `agent-workflows approve WORK_ID GATE REVISION` with the full Work +Item revision is still accepted. The worker imports permitted approval during polling. To import existing comments without waiting for another poll, run the sync command. diff --git a/docs/operations.md b/docs/operations.md index c610e08..4ed4d2f 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -31,6 +31,7 @@ All commands accept `--config PATH` before the subcommand. For example, | `retry RUN` | Request a new attempt after checkout validation | | `retry RUN --refresh-issue` | Request a new attempt using the current hosted issue | | `monitor [--notify]` | Attach the TUI; optionally enable outcome notifications | +| `monitor --web [WORK_ID]` | Open the running web monitor with its access link, optionally at a Work Item | Version reporting requires no configuration, credentials, or database connection. @@ -47,15 +48,19 @@ Use arrows to navigate and Enter or Space to expand or collapse; **Back to runs* returns to evidence. This view refreshes after accepted reloads and labels retained data stale when the active snapshot cannot be read. -`run` starts a separate web-server process on `127.0.0.1:8787`. Use -`run --web-port 8899` to select another port or `run --no-web` to disable it. -An occupied port or a web-server failure produces a warning while workflows -continue; the CLI does not switch ports or automatically open a browser. +`run` starts a separate web-server process on `127.0.0.1:8787`, or on a free +port when 8787 is taken, and says so. Use `run --web-port 8899` to require a +port or `run --no-web` to disable it. An occupied required port or a web-server +failure produces a warning while workflows continue. `run` does not open a +browser; `agent-workflows monitor --web` opens the current link, and +`monitor --web WORK_ID` opens a Work Item. Links ending in `#work=WORK_ID` +select that Work Item in a browser that already has access. Open the exact printed URL. Its launch-specific token grants access without a login or token-entry step; the browser then removes the token from the address bar and uses an HTTP-only session cookie. Treat the printed link as local access -credentials. After restarting `run`, open its new link. The server accepts only +credentials. The runner keeps it in its mode-600 control record so +`monitor --web` can reopen it. After restarting `run`, open its new link. The server accepts only localhost access and its own browser origin for actions; remote hosting is not supported. diff --git a/packages/cli/src/cli-runner-control.ts b/packages/cli/src/cli-runner-control.ts index f7bd76a..b26da6c 100644 --- a/packages/cli/src/cli-runner-control.ts +++ b/packages/cli/src/cli-runner-control.ts @@ -24,6 +24,8 @@ const controlSchema = z.strictObject({ pid: z.number().int().positive(), nonce: z.string(), configuration: configSchema.optional(), + /** Web monitor access URL, including its launch token. */ + webUrl: z.url().optional(), }); type RunnerControl = z.infer; @@ -115,13 +117,31 @@ export async function publishActiveConfiguration( configPath: string, configuration: Configuration, ): Promise { - const path = controlPath(configPath); const control = await readRunnerControl(configPath); if (control.pid !== process.pid || control.id !== configuration.id) throw new Error("Cannot publish another runner's configuration"); + await replaceControl(configPath, { ...control, configuration }); +} + +/** Record the web monitor link so `monitor --web` can reopen it later. */ +export async function publishWebUrl( + configPath: string, + webUrl: string, +): Promise { + const control = await readRunnerControl(configPath); + if (control.pid !== process.pid) + throw new Error("Cannot publish another runner's web monitor"); + await replaceControl(configPath, { ...control, webUrl }); +} + +async function replaceControl( + configPath: string, + record: RunnerControl, +): Promise { + const path = controlPath(configPath); const temporary = `${path}.${randomUUID()}.tmp`; try { - await writeFile(temporary, JSON.stringify({ ...control, configuration }), { + await writeFile(temporary, JSON.stringify(record), { flag: "wx", mode: 0o600, }); @@ -131,6 +151,11 @@ export async function publishActiveConfiguration( } } +/** The web monitor's address without its access token. */ +export function monitorAddress(webUrl: string): string { + return new URL(webUrl).origin + "/"; +} + export async function findRunnerControl( configPath: string, ): Promise { diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index 70ad38b..1ca0de8 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -1,5 +1,6 @@ #!/usr/bin/env -S node -- +import { spawn } from "node:child_process"; import { realpathSync, statSync } from "node:fs"; import { readFile, writeFile } from "node:fs/promises"; import { basename, dirname, relative, resolve } from "node:path"; @@ -25,8 +26,10 @@ import { } from "./cli-config.js"; import { findRunnerControl, + monitorAddress, publishActiveConfiguration, publishRunnerControl, + publishWebUrl, readActiveConfiguration, readRunnerControl, } from "./cli-runner-control.js"; @@ -108,6 +111,13 @@ function databaseUrl(config: Pick): string { throw new Error(`Set ${config.databaseUrlEnv} to a PostgreSQL URL`); return value; } +function openBrowser(url: string): void { + const opener = process.platform === "darwin" ? "open" : "xdg-open"; + // Opening is a convenience; the printed link still works. + spawn(opener, [url], { stdio: "ignore", detached: true }) + .on("error", () => undefined) + .unref(); +} async function withStore(action: (store: Store) => Promise) { const config = await configuration(); await withConfiguredStore(config, action); @@ -208,10 +218,13 @@ program .description("Start the worker and the web monitor; keep it running") .option("-p, --project ", "run selected project IDs") .option("--no-web", "disable the local web monitor") - .option("--web-port ", "local web monitor port", String(defaultWebPort)) + .option( + "--web-port ", + `local web monitor port (default: ${defaultWebPort}, or a free port when it is taken)`, + ) .action( - async (options: { project?: string[]; web: boolean; webPort: string }) => { - const webPort = Number(options.webPort); + async (options: { project?: string[]; web: boolean; webPort?: string }) => { + const webPort = Number(options.webPort ?? defaultWebPort); if (!Number.isInteger(webPort) || webPort < 1 || webPort > 65535) throw new Error("--web-port must be an integer between 1 and 65535"); const path = configPath(); @@ -266,6 +279,9 @@ program configPath: path, databaseUrl: databaseUrl(config), port: webPort, + fallbackPort: options.webPort === undefined, + onReady: (url) => + void publishWebUrl(path, url).catch(() => undefined), }); } catch { console.error( @@ -437,8 +453,25 @@ program .command("monitor") .description("Open the terminal monitor: runs, Work Items and approvals") .option("--notify", "notify when an observed execution reaches an outcome") - .action(async (options: { notify?: boolean }) => { + .option( + "--web [workItem]", + "open the running web monitor instead, optionally at a Work Item", + ) + .action(async (options: { notify?: boolean; web?: boolean | string }) => { const control = await findRunnerControl(configPath()); + if (options.web) { + if (!control?.webUrl) + throw new Error( + "No web monitor is running for this configuration; start agent-workflows run", + ); + const url = + typeof options.web === "string" + ? `${control.webUrl}#work=${encodeURIComponent(options.web)}` + : control.webUrl; + console.log(`Web monitor: ${url}`); + if (process.stdout.isTTY) openBrowser(url); + return; + } if (control?.envFile) await loadEnvironmentFile(control.envFile); const config = control?.configuration ?? (await configuration()); if (control && sha256(databaseUrl(config)) !== control.databaseUrlHash) @@ -491,6 +524,10 @@ program registerWorkCommands(program, { pathBaseDirectory: configBaseDirectory, configuration, + monitorUrl: async () => { + const webUrl = (await findRunnerControl(configPath()))?.webUrl; + return webUrl && monitorAddress(webUrl); + }, withStore: withConfiguredStore, }); // Help lists groups in command order, so daily commands come first. diff --git a/packages/cli/src/doctor.ts b/packages/cli/src/doctor.ts index 777eafb..f473eaa 100644 --- a/packages/cli/src/doctor.ts +++ b/packages/cli/src/doctor.ts @@ -6,7 +6,7 @@ import { Store, } from "@mingchuno/agent-workflows-core"; import type { Command } from "commander"; -import { findRunnerControl } from "./cli-runner-control.js"; +import { findRunnerControl, monitorAddress } from "./cli-runner-control.js"; import { withinCheckout } from "./config-discovery.js"; import { applyCliTokens, cliTokenAllowed } from "./hosting-token.js"; import { onPath, remoteRepository } from "./repository-inference.js"; @@ -22,6 +22,8 @@ export interface DoctorReport { version: string; configPath?: string; project?: string; + /** The running web monitor, without its access token. */ + monitorUrl?: string; protocols: { inline: true; delegated: boolean }; checks: Check[]; } @@ -156,6 +158,7 @@ export async function doctor( const checks: Check[] = []; const check = (name: string, status: Status, detail: string) => checks.push({ name, status, detail }); + let monitorUrl: string | undefined; const report = (project?: string): DoctorReport => { let configPath: string | undefined; try { @@ -169,6 +172,7 @@ export async function doctor( version: dependencies.version, configPath, project, + ...(monitorUrl ? { monitorUrl } : {}), protocols: { inline: true, delegated: ok }, checks, }; @@ -274,11 +278,12 @@ export async function doctor( check("database", "fail", message(error)); } const worker = await findRunnerControl(dependencies.configPath()); + if (worker?.webUrl) monitorUrl = monitorAddress(worker.webUrl); check( "worker", worker ? "pass" : "warn", worker - ? `Running as process ${worker.pid}` + ? `Running as process ${worker.pid}${monitorUrl ? `; web monitor ${monitorUrl}` : ""}` : "Not running. Start `agent-workflows run` in its own terminal; approved work waits until then.", ); return report(project.id); diff --git a/packages/cli/src/tui/work-items.tsx b/packages/cli/src/tui/work-items.tsx index 83eeb39..07f974e 100644 --- a/packages/cli/src/tui/work-items.tsx +++ b/packages/cli/src/tui/work-items.tsx @@ -94,6 +94,9 @@ export function WorkItemsView({ ...(selected.approvalSource !== "local-operator" ? [ `Approval requires an authenticated allowed human through ${selected.approvalSource}.`, + ...(selected.approvalComment + ? [`Comment on the issue: ${selected.approvalComment}`] + : []), ] : []), "", diff --git a/packages/cli/src/web/lifecycle.ts b/packages/cli/src/web/lifecycle.ts index 022bc96..d8e08fe 100644 --- a/packages/cli/src/web/lifecycle.ts +++ b/packages/cli/src/web/lifecycle.ts @@ -10,6 +10,8 @@ export interface WebStartup { stateDirectory: string; projectIds: string[]; port: number; + /** Use any free port when `port` is the taken default. */ + fallbackPort?: boolean; token: string; } @@ -19,6 +21,9 @@ export function startWebMonitor(options: { configPath: string; databaseUrl: string; port: number; + fallbackPort?: boolean; + /** Receives the access URL once the server listens. */ + onReady?: (url: string) => void; }) { const token = randomBytes(32).toString("hex"); const redact = createRedactor([token, options.databaseUrl]); @@ -46,10 +51,17 @@ export function startWebMonitor(options: { if (!message || typeof message !== "object") return; if ("ready" in message && message.ready === true) { clearTimeout(timer); - if (!stopping) - console.log( - `Web monitor: http://127.0.0.1:${options.port}/?token=${token}`, - ); + const port = + "port" in message && typeof message.port === "number" + ? message.port + : options.port; + const url = `http://127.0.0.1:${port}/?token=${token}`; + if (!stopping) { + if (port !== options.port) + console.log(`Port ${options.port} is in use; using ${port}.`); + console.log(`Web monitor: ${url}`); + options.onReady?.(url); + } } else if ("error" in message && typeof message.error === "string") { clearTimeout(timer); warn(message.error); @@ -76,6 +88,7 @@ export function startWebMonitor(options: { stateDirectory: options.config.stateDirectory, projectIds: options.config.projects.map((project) => project.id), port: options.port, + fallbackPort: options.fallbackPort ?? false, token, }; child.send(startup, (error) => { diff --git a/packages/cli/src/web/server.ts b/packages/cli/src/web/server.ts index ed02e79..110e89e 100644 --- a/packages/cli/src/web/server.ts +++ b/packages/cli/src/web/server.ts @@ -83,7 +83,7 @@ export function monitorServer(store: Store, startup: WebStartup) { if (token !== undefined) { if (!sameSecret(token, startup.token)) return context.text( - "Access link is invalid. Open the URL printed by run.", + "Access link is invalid. Run agent-workflows monitor --web for the current link.", 401, ); setCookie(context, cookieName, session, { @@ -98,7 +98,10 @@ export function monitorServer(store: Store, startup: WebStartup) { app.use("/api/*", async (context, next) => { if (!sameSecret(getCookie(context, cookieName), session)) return context.json( - { error: "Open the access URL printed by run to connect." }, + { + error: + "Run agent-workflows monitor --web to open the monitor with access.", + }, 401, ); await next(); diff --git a/packages/cli/src/web/worker.ts b/packages/cli/src/web/worker.ts index d3bee49..1c1ece3 100644 --- a/packages/cli/src/web/worker.ts +++ b/packages/cli/src/web/worker.ts @@ -1,4 +1,5 @@ import { access } from "node:fs/promises"; +import { createServer } from "node:net"; import { join } from "node:path"; import { serve } from "@hono/node-server"; import { Store } from "@mingchuno/agent-workflows-core"; @@ -7,6 +8,23 @@ import { webAssetDirectory } from "./assets.js"; import type { WebStartup } from "./lifecycle.js"; import { monitorServer } from "./server.js"; +/** The requested port, or any free one when the default port is taken. */ +function choosePort(port: number, fallback: boolean): Promise { + if (!fallback) return Promise.resolve(port); + return new Promise((done) => { + const probe = createServer(); + probe.once("error", () => { + const free = createServer(); + free.once("error", () => done(port)); + free.listen(0, "127.0.0.1", () => { + const { port: chosen } = free.address() as { port: number }; + free.close(() => done(chosen)); + }); + }); + probe.listen(port, "127.0.0.1", () => probe.close(() => done(port))); + }); +} + let close: (() => Promise) | undefined; let stopping = false; const stop = async () => { @@ -22,8 +40,13 @@ const stop = async () => { process.once("disconnect", () => void stop()); process.once("SIGTERM", () => void stop()); process.once("SIGINT", () => void stop()); -process.once("message", (startup: WebStartup) => { +process.once("message", (requested: WebStartup) => { + let startup = requested; void (async () => { + startup = { + ...requested, + port: await choosePort(requested.port, requested.fallbackPort ?? false), + }; const store = new Store(startup.databaseUrl, startup.scope); close = () => store.close(); await access(join(webAssetDirectory, "index.html")); @@ -33,7 +56,8 @@ process.once("message", (startup: WebStartup) => { const server = serve( { fetch: app.fetch, hostname: "127.0.0.1", port: startup.port }, () => { - if (process.connected) process.send?.({ ready: true }); + if (process.connected) + process.send?.({ ready: true, port: startup.port }); }, ); close = async () => { diff --git a/packages/cli/src/work-commands.ts b/packages/cli/src/work-commands.ts index a4e02e1..190afa9 100644 --- a/packages/cli/src/work-commands.ts +++ b/packages/cli/src/work-commands.ts @@ -15,6 +15,7 @@ import { publishWork, type Store, syncWorkApprovals, + type WorkInspection, workContractSchema, workOperator, } from "@mingchuno/agent-workflows-core"; @@ -23,6 +24,8 @@ import type { Command } from "commander"; interface WorkCommandDependencies { pathBaseDirectory(): string; configuration(): Promise; + /** The running web monitor address without its token, if any. */ + monitorUrl(): Promise; withStore( config: Configuration, action: (store: Store) => Promise, @@ -67,9 +70,17 @@ export function registerWorkCommands( throw new Error("Enable sdlc for this project first"); await dependencies.withStore(config, async (store) => { await store.registerProject(project.id); - console.log( - JSON.stringify(await action(store, project, config), null, 2), - ); + const result = await action(store, project, config); + // Point the human at the Work Item in the web monitor, if it runs. + const monitor = await dependencies.monitorUrl(); + const linked = + monitor && result && typeof result === "object" && "work" in result + ? { + ...result, + monitorUrl: `${monitor}#work=${encodeURIComponent((result as WorkInspection).work.id)}`, + } + : result; + console.log(JSON.stringify(linked, null, 2)); }); }; const command = (syntax: string) => diff --git a/packages/cli/web/app.tsx b/packages/cli/web/app.tsx index 91ea31b..3c39515 100644 --- a/packages/cli/web/app.tsx +++ b/packages/cli/web/app.tsx @@ -12,9 +12,16 @@ import { RunWorkspace } from "./workspace.js"; const fixturePreview = import.meta.env.VITE_MONITOR_FIXTURES === "true"; +/** `#work=ID` opens a Work Item, as linked by start-work and `monitor --web`. */ +function linkedWorkId(): string | undefined { + const value = new URLSearchParams(window.location.hash.slice(1)).get("work"); + return value || undefined; +} + export function App() { + const [linkedWork] = useState(linkedWorkId); const [view, setView] = useState<"projects" | "configuration" | "work-items">( - "projects", + linkedWork ? "work-items" : "projects", ); const showConfiguration = view === "configuration"; const [runPickerExpanded, setRunPickerExpanded] = useState(false); @@ -265,7 +272,11 @@ export function App() { onReload={() => commands.submit({ kind: "reload", target: "" })} /> ) : view === "work-items" ? ( - + ) : run ? ( (); + const [selectedId, setSelectedId] = useState(linkedWorkId); const [confirmation, setConfirmation] = useState<{ id: string; title: string; @@ -34,7 +37,10 @@ export function WorkItemsView({ }); const items = snapshot.data?.items.filter( - (item) => !projectId || item.work.projectId === projectId, + (item) => + !projectId || + item.work.projectId === projectId || + item.work.id === linkedWorkId, ) ?? []; const selected = items.find((item) => item.work.id === selectedId) ?? items[0]; @@ -195,11 +201,18 @@ export function WorkItemsView({ {JSON.stringify(selected.work.contract, null, 2)} {selected.approvalSource !== "local-operator" ? ( -

- This project requires authenticated approval from an allowed human - through {selected.approvalSource}. Local monitor actions cannot - grant that approval. -

+
+

+ This project requires authenticated approval from an allowed + human through {selected.approvalSource}. Local monitor actions + cannot grant that approval. +

+ {selected.approvalComment && ( +

+ Comment on the issue: {selected.approvalComment} +

+ )} +
) : gate && !confirmation ? (