From 1e1bd52d1a870dff6b9617446783581877e1dba6 Mon Sep 17 00:00:00 2001 From: karim Date: Sat, 17 Oct 2026 18:01:56 +0200 Subject: [PATCH 1/8] Local Publish decides its own next step, measured first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Measurement first: a labelled set from the stream's records (failed checks, reviews with findings, CI failures of PRs #91–#124) measured over three deciders — Jev via ai.decision.decide, critique-then-verdict gpt-6-luna, critique-then-verdict claude-opus-5-5 — on agreement, abstention at the chosen minConfidence, latency and cost; recorded in Plan 316's measurement section (yarn decisions:measure-loop) - After a failed check: one choice among recheck_on_timeout, fix_commission, operator_install_mismatch; onUndecided = today's failure, which wakes Helmsman; the fix brief assembled from the check's output tail and the failing guard's name, no prose author - After a review with findings: one yes_no per finding and a choice among fix_before_merge, publish_as_is, ask; minConfidence high, onUndecided = the fix round; the fix brief assembled from the findings that passed - The push gate, the scan and the posture untouched (316 D9); tests route planted check outputs and finding lists through each route and the undecided branch; Plan 315 §2 gains the decision, Plan 316's status names the first production consumer --- .../src/handlers/ai/handlers/decide.test.ts | 83 +++ .../src/handlers/ai/handlers/decide.ts | 31 +- .../src/__tests__/commitCheck.test.ts | 35 + apps/aflow-executor-host/src/commitCheck.ts | 17 +- .../__tests__/workflowRunReadOps.test.ts | 30 + .../workflowRunStart.fromTask.test.ts | 8 +- .../inlineOps/workflowCrud/handleInline.ts | 16 + .../helpers/inlineOperations.ts | 2 + docs/dev/driving-work-through-the-loop.md | 29 +- docs/plans/315-local-first-run-ergonomics.md | 2 + ...rkflows-decide-without-a-language-model.md | 36 +- package.json | 1 + packages/ai-client/src/decision.test.ts | 67 +- packages/ai-client/src/decision.ts | 74 +- packages/ai-client/src/index.ts | 6 +- packages/platform-artifacts/src/index.ts | 10 + .../skillCatalog/publishLocalChanges.test.ts | 694 ++++++++++++++++-- .../src/skillCatalog/publishLocalChanges.ts | 123 ++-- .../publishLocalChangesDecisions.ts | 84 +++ .../skillCatalog/publishLocalChangesRounds.ts | 445 +++++++++++ .../skillCatalog/reviewLocalChanges.test.ts | 8 +- .../src/skillCatalog/reviewLocalChanges.ts | 12 +- .../src/storeCatalogHashes.json | 4 +- .../schemas/src/operations/aiDecision.test.ts | 29 + packages/schemas/src/operations/aiDecision.ts | 108 ++- .../schemas/src/operations/hostCommitCheck.ts | 13 + .../schemas/src/operations/workflow/index.ts | 4 + .../src/operations/workflow/registrations.ts | 44 ++ .../src/operations/workflow/runFailOp.ts | 24 + .../schemas/src/runtime/parentInstructions.ts | 9 +- scripts/decisions-measure-loop.ts | 473 ++++++++++++ scripts/loop-decision-set.mjs | 321 ++++++++ scripts/loop-decision-set.test.mjs | 237 ++++++ scripts/verify-commit.mjs | 20 +- 34 files changed, 2951 insertions(+), 148 deletions(-) create mode 100644 apps/aflow-executor-ai/src/handlers/ai/handlers/decide.test.ts create mode 100644 packages/platform-artifacts/src/skillCatalog/publishLocalChangesDecisions.ts create mode 100644 packages/platform-artifacts/src/skillCatalog/publishLocalChangesRounds.ts create mode 100644 packages/schemas/src/operations/workflow/runFailOp.ts create mode 100644 scripts/decisions-measure-loop.ts create mode 100644 scripts/loop-decision-set.mjs create mode 100644 scripts/loop-decision-set.test.mjs diff --git a/apps/aflow-executor-ai/src/handlers/ai/handlers/decide.test.ts b/apps/aflow-executor-ai/src/handlers/ai/handlers/decide.test.ts new file mode 100644 index 00000000..cfb482f6 --- /dev/null +++ b/apps/aflow-executor-ai/src/handlers/ai/handlers/decide.test.ts @@ -0,0 +1,83 @@ +/** + * ai.decision.decide as a step: the questions a request asks reach the model + * as one pass, and an `each` over an empty list is answered without one. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { ExecutorContext } from '@aflow/executor-runtime'; +import { AiDecideOutputSchema, type AiDecideInput, type TenantId } from '@aflow/schemas'; +import { getAIClientForContext } from '../aiClient.js'; +import { handleDecide } from './decide.js'; +import type { HandlerDeps } from './types.js'; + +vi.mock('../aiClient.js', () => ({ getAIClientForContext: vi.fn() })); +vi.mock('@aflow/executor-runtime', async (importOriginal) => ({ + ...(await importOriginal()), + successWithData: vi.fn((_ctx: unknown, data: unknown) => + Promise.resolve({ status: 'SUCCEEDED', data }), + ), +})); + +const decide = vi.fn(); + +function fakeCtx(): ExecutorContext { + return { + job: { tenantId: 'tenant-d', stepExecutionId: 'step-d', attempt: 1 }, + tenantId: 'tenant-d' as TenantId, + runId: 'run-d', + log: { warn: () => {}, error: () => {}, info: () => {}, debug: () => {} }, + } as unknown as ExecutorContext; +} + +const deps = { + payloadStore: {} as HandlerDeps['payloadStore'], + handleError: (_ctx: ExecutorContext, label: string, error: unknown) => { + throw new Error(`${label}: ${String(error)}`); + }, + validateToolArgs: () => null, +} satisfies HandlerDeps; + +const FINDING = { + type: 'yes_no' as const, + instructions: 'Names a defect, established by evidence shown', + minConfidence: 0.8, +}; + +async function run(input: AiDecideInput) { + const result = (await handleDecide(fakeCtx(), input, deps)) as unknown as { data: unknown }; + return AiDecideOutputSchema.parse(result.data); +} + +describe('ai.decision.decide — a question of each item', () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(getAIClientForContext).mockResolvedValue({ decide } as never); + }); + + it('asks every item in the one call, and hands back only the items judged true and decided', async () => { + decide.mockResolvedValue({ + model: 'jev-1.13.0', + answers: { + each_0: { type: 'yes_no', probability: 0.97 }, + each_1: { type: 'yes_no', probability: 0.6 }, + }, + usage: { promptTokens: 10, completionTokens: 0, totalTokens: 10 }, + }); + const output = await run({ + state: { verdict: 'request_changes' }, + each: { items: [{ claim: 'leaks' }, { claim: 'maybe' }], question: FINDING }, + }); + expect(decide).toHaveBeenCalledOnce(); + expect(Object.keys(decide.mock.calls[0]![0].questions as object)).toEqual(['each_0', 'each_1']); + expect(output.answers).toEqual({}); + expect(output.each?.selected).toEqual([{ claim: 'leaks' }]); + expect(output.each?.undecidedCount).toBe(1); + }); + + it('asks no model, and needs no credential, for an empty list — and selects nothing', async () => { + const output = await run({ state: {}, each: { items: [], question: FINDING } }); + expect(getAIClientForContext).not.toHaveBeenCalled(); + expect(decide).not.toHaveBeenCalled(); + expect(output.each).toMatchObject({ selected: [], selectedCount: 0, selectedJson: '[]' }); + expect(output.usage.totalTokens).toBe(0); + }); +}); diff --git a/apps/aflow-executor-ai/src/handlers/ai/handlers/decide.ts b/apps/aflow-executor-ai/src/handlers/ai/handlers/decide.ts index 96f285b4..be22ea06 100644 --- a/apps/aflow-executor-ai/src/handlers/ai/handlers/decide.ts +++ b/apps/aflow-executor-ai/src/handlers/ai/handlers/decide.ts @@ -4,7 +4,12 @@ import type { ExecutorContext, StepResult } from '@aflow/executor-runtime'; import { successWithData } from '@aflow/executor-runtime'; import type { AiDecideInput, AiDecideOutput, TenantId, StepExecutionId } from '@aflow/schemas'; -import { DEFAULT_AI_MODELS, resolveDecisionAnswers } from '@aflow/ai-client'; +import { + DEFAULT_AI_MODELS, + decisionRequestQuestions, + resolveDecisionAnswers, + splitDecisionAnswers, +} from '@aflow/ai-client'; import { getAIClientForContext } from '../aiClient.js'; import { buildUsageBreakdown } from '../helpers.js'; import type { HandlerDeps } from './types.js'; @@ -16,21 +21,39 @@ export async function handleDecide( ): Promise { try { const model = params.model ?? DEFAULT_AI_MODELS.decision; + const questions = decisionRequestQuestions(params); + // Only an empty `each` asks nothing; the answer is known without a model, + // and so without the credential one would need. + if (Object.keys(questions).length === 0) { + const { answers, each } = splitDecisionAnswers(params, {}); + const output: AiDecideOutput = { + answers, + ...(each !== undefined ? { each } : {}), + model, + usage: { promptTokens: 0, completionTokens: 0, totalTokens: 0 }, + latencyMs: 0, + }; + return await successWithData(ctx, output); + } const client = await getAIClientForContext(ctx, model); - const startedAt = Date.now(); const response = await client.decide({ model, state: params.state, - questions: params.questions, + questions, tenantId: ctx.job.tenantId as TenantId, runId: ctx.runId, stepExecutionId: ctx.job.stepExecutionId as StepExecutionId, attempt: ctx.job.attempt, }); + const { answers, each } = splitDecisionAnswers( + params, + resolveDecisionAnswers(questions, response.answers), + ); const output: AiDecideOutput = { - answers: resolveDecisionAnswers(params.questions, response.answers), + answers, + ...(each !== undefined ? { each } : {}), model: response.model, usage: response.usage, latencyMs: Date.now() - startedAt, diff --git a/apps/aflow-executor-host/src/__tests__/commitCheck.test.ts b/apps/aflow-executor-host/src/__tests__/commitCheck.test.ts index a078ac0f..e1af3043 100644 --- a/apps/aflow-executor-host/src/__tests__/commitCheck.test.ts +++ b/apps/aflow-executor-host/src/__tests__/commitCheck.test.ts @@ -634,6 +634,41 @@ describe('host.commit.check — a run lost only to tests running out of time', ( }); }); +describe('host.commit.check — the step the checks name as failing', () => { + /** A check whose report names `guard` as the failing step, and ends as `exit` says. */ + function reportingGuard(guard: string, exit: string): string[] { + return nodeCheck(`${reportScript(JSON.stringify({ failingGuard: guard }))}\n${exit}`); + } + + it('names the failing step on checks that failed, from a report that carries no listener count', async () => { + const world = await fixture({ + branchPrefix: 'aflow/', + checks: reportingGuard('tsc apps/x/tsconfig.json', 'process.exit(2);'), + }); + const output = HostCommitCheckOutputSchema.parse((await check(world)).captured.output); + expect(output.passed).toBe(false); + expect(output.failingGuard).toBe('tsc apps/x/tsconfig.json'); + // The summary is what a fix is briefed with, so it names the step too. + expect(output.summary).toContain('the check failed at `tsc apps/x/tsconfig.json`.'); + // Unknown, not zero: the run failed before the tests that would count them. + expect(output.skippedListenerTests).toBeUndefined(); + expect(output.receipt).toBeDefined(); + }); + + it('names none on checks that passed, whatever the report says, nor where the checks named none', async () => { + const passing = await fixture({ + branchPrefix: 'aflow/', + checks: reportingGuard('tsc apps/x/tsconfig.json', 'process.exit(0);'), + }); + const passed = HostCommitCheckOutputSchema.parse((await check(passing)).captured.output); + expect(passed.failingGuard).toBeUndefined(); + + const silent = await fixture({ branchPrefix: 'aflow/', checks: FAILING }); + const failed = HostCommitCheckOutputSchema.parse((await check(silent)).captured.output); + expect(failed.failingGuard).toBeUndefined(); + }); +}); + describe("host.commit.check — the folder's sandbox posture", () => { function compiled(input: SandboxedRunInput): ReturnType { return compileSandboxPolicy(input.binding, { diff --git a/apps/aflow-executor-host/src/commitCheck.ts b/apps/aflow-executor-host/src/commitCheck.ts index ec6e62c0..be56d3c9 100644 --- a/apps/aflow-executor-host/src/commitCheck.ts +++ b/apps/aflow-executor-host/src/commitCheck.ts @@ -17,6 +17,7 @@ import { HOST_CHECK_OUTPUT_HEAD_BYTES, HOST_CHECK_OUTPUT_TAIL_BYTES, HOST_CHECK_TAIL_BYTES, + HOST_CHECK_FAILING_GUARD_MAX_CHARS, HOST_CHECK_TEST_TIMEOUTS_MAX, type HostCommitCheckOutputSchema, type HostSandboxPosture, @@ -162,8 +163,9 @@ export const CHECK_REPORT_ENV = 'AFLOW_CHECK_REPORT'; const CHECK_REPORT_FILE = 'check-report.json'; const CheckReportSchema = z.object({ - skippedListenerTests: z.number().int().nonnegative(), + skippedListenerTests: z.number().int().nonnegative().optional(), testTimeouts: z.array(z.string()).optional(), + failingGuard: z.string().min(1).max(HOST_CHECK_FAILING_GUARD_MAX_CHARS).optional(), }); /** What the check wrote to its report, or nothing where it wrote none a reader can take. */ @@ -194,6 +196,8 @@ export interface FolderCheckRun { readonly skippedListenerTests?: number; /** The tests that ran out of their time, where its report says they alone failed it. */ readonly testTimeouts?: readonly string[]; + /** The step the check names as the one that failed, where its report says. */ + readonly failingGuard?: string; } export interface FolderCheckInput { @@ -270,7 +274,10 @@ export async function runFolderChecks(input: FolderCheckInput): Promise 0 ? { testTimeouts: report.testTimeouts } : {}), @@ -347,7 +354,8 @@ export function checkOutcome(params: { result.exitCode === null ? `was ended by ${result.signal ?? 'a signal'}` : `exited ${String(result.exitCode)}`; - summary = `${command} ${ending} on ${short} after ${seconds(result.durationMs)}: the check failed.${printed}`; + const at = params.run.failingGuard !== undefined ? ` at \`${params.run.failingGuard}\`` : ''; + summary = `${command} ${ending} on ${short} after ${seconds(result.durationMs)}: the check failed${at}.${printed}`; } return { passed, @@ -365,6 +373,9 @@ export function checkOutcome(params: { ...(testTimeouts.length > 0 ? { testTimeouts: testTimeouts.slice(0, HOST_CHECK_TEST_TIMEOUTS_MAX) } : {}), + ...(!passed && params.run.failingGuard !== undefined + ? { failingGuard: params.run.failingGuard } + : {}), ...(passed ? { clearedSha: params.sha } : {}), ...(passed || testTimeouts.length > 0 ? { clearedOrRetriableSha: params.sha } : {}), }; diff --git a/apps/aflow-orchestrator/src/services/SessionOrchestrator/handlers/inlineOps/__tests__/workflowRunReadOps.test.ts b/apps/aflow-orchestrator/src/services/SessionOrchestrator/handlers/inlineOps/__tests__/workflowRunReadOps.test.ts index 149922d9..86ba02cc 100644 --- a/apps/aflow-orchestrator/src/services/SessionOrchestrator/handlers/inlineOps/__tests__/workflowRunReadOps.test.ts +++ b/apps/aflow-orchestrator/src/services/SessionOrchestrator/handlers/inlineOps/__tests__/workflowRunReadOps.test.ts @@ -911,3 +911,33 @@ describe('workflow.run.detail — one task’s output on request (Plan 313 gap 1 expect(output.taskOutput).toBeUndefined(); }); }); + +describe('workflow.run.fail — a skill stops its run on what decided it', () => { + type Failed = { + status: string; + error: { code: string; message: string; classification: string; retryable: boolean }; + }; + + it('always fails its own task, non-retryable, with the reason it was handed as the message', async () => { + const reason = + 'The checks failed at `tsc apps/x/tsconfig.json`. The last of what they printed: …'; + await handleWorkflowCrudInline(makeArgs('workflow.run.fail', { reason })); + + expect(mockAddStepResult).toHaveBeenCalledOnce(); + const result = mockAddStepResult.mock.calls[0]![1] as Failed; + expect(result.status).toBe('FAILED'); + expect(result.error.code).toBe('RUN_FAILED_BY_SKILL'); + // Never the internal-error message: the reason is what the woken reader acts on. + expect(result.error.message).toBe(reason); + expect(result.error.classification).not.toBe('internal'); + expect(result.error.retryable).toBe(false); + }); + + it('still fails, saying so, when it was handed no reason', async () => { + await handleWorkflowCrudInline(makeArgs('workflow.run.fail', {})); + + const result = mockAddStepResult.mock.calls[0]![1] as Failed; + expect(result.status).toBe('FAILED'); + expect(result.error.message).toContain('without a reason'); + }); +}); diff --git a/apps/aflow-orchestrator/src/services/SessionOrchestrator/handlers/inlineOps/__tests__/workflowRunStart.fromTask.test.ts b/apps/aflow-orchestrator/src/services/SessionOrchestrator/handlers/inlineOps/__tests__/workflowRunStart.fromTask.test.ts index 16eb4c69..a5267d09 100644 --- a/apps/aflow-orchestrator/src/services/SessionOrchestrator/handlers/inlineOps/__tests__/workflowRunStart.fromTask.test.ts +++ b/apps/aflow-orchestrator/src/services/SessionOrchestrator/handlers/inlineOps/__tests__/workflowRunStart.fromTask.test.ts @@ -334,7 +334,7 @@ describe('workflow.run.start from a workflow task', () => { mockLoadRunById.mockResolvedValue({ runId: PARENT_RUN, sessionId: DRIVER, - metadata: { runDepth: 2 }, + metadata: { runDepth: 3 }, }); await handleWorkflowCrudInline(makeTaskArgs('until_complete')); @@ -346,9 +346,9 @@ describe('workflow.run.start from a workflow task', () => { >; expect(result.status).toBe('FAILED'); expect(result.error?.code).toBe('WORKFLOW_RUN_DEPTH_EXCEEDED'); - expect(result.error?.message).toContain('at depth 2'); - expect(result.error?.message).toContain('at most 2 deep'); - expect(result.error?.message).toContain('"lead-scoring" at depth 3'); + expect(result.error?.message).toContain('at depth 3'); + expect(result.error?.message).toContain('at most 3 deep'); + expect(result.error?.message).toContain('"lead-scoring" at depth 4'); }); it('refuses a workflow that is not the catalog skill it was named as, with the reason', async () => { diff --git a/apps/aflow-orchestrator/src/services/SessionOrchestrator/handlers/inlineOps/workflowCrud/handleInline.ts b/apps/aflow-orchestrator/src/services/SessionOrchestrator/handlers/inlineOps/workflowCrud/handleInline.ts index 854b52ae..186c1e53 100644 --- a/apps/aflow-orchestrator/src/services/SessionOrchestrator/handlers/inlineOps/workflowCrud/handleInline.ts +++ b/apps/aflow-orchestrator/src/services/SessionOrchestrator/handlers/inlineOps/workflowCrud/handleInline.ts @@ -18,6 +18,7 @@ import type { WorkflowCampaignEndInput, WorkflowCampaignRefreshInput, } from '@aflow/schemas'; +import { WorkflowRunFailInputSchema } from '@aflow/schemas'; import { WorkflowArchivedError } from '@aflow/cybernetic-runtime'; import { getOrchestratorLogger } from '../../../../../lib/orchestratorLogger.js'; import type { InlineHandlerArgs } from '../types.js'; @@ -76,6 +77,21 @@ export async function handleWorkflowCrudInline(args: InlineHandlerArgs): Promise case 'workflow.run.cancel': await handleWorkflowRunCancel(args, input as unknown as WorkflowRunCancelInput, startTime); break; + case 'workflow.run.fail': { + // A skill's decision to stop: the reason is what the reader of the + // failure needs, so it is never replaced by the internal-error message. + const parsed = WorkflowRunFailInputSchema.safeParse(input); + await emitStepError( + args, + 'RUN_FAILED_BY_SKILL', + parsed.success + ? parsed.data.reason + : 'The skill stopped the run here without a reason it could state.', + startTime, + 'validation', + ); + break; + } case 'workflow.run.detail': await handleWorkflowRunDetail(args, input as unknown as WorkflowRunDetailInput, startTime); break; diff --git a/apps/aflow-orchestrator/src/services/SessionOrchestrator/helpers/inlineOperations.ts b/apps/aflow-orchestrator/src/services/SessionOrchestrator/helpers/inlineOperations.ts index 00a147fb..80e95814 100644 --- a/apps/aflow-orchestrator/src/services/SessionOrchestrator/helpers/inlineOperations.ts +++ b/apps/aflow-orchestrator/src/services/SessionOrchestrator/helpers/inlineOperations.ts @@ -65,6 +65,8 @@ export const WORKFLOW_TASK_SAFE_INLINE_OPERATIONS: ReadonlySet = new Set 'workflow.learn', // Waits on the run it starts, no deeper than MAX_WORKFLOW_RUN_DEPTH; the waiter answers the task 'workflow.run.start', + // Always fails its own task, with the reason it was handed + 'workflow.run.fail', 'agent.schedule.snooze', ]); diff --git a/docs/dev/driving-work-through-the-loop.md b/docs/dev/driving-work-through-the-loop.md index 2cd750fe..0fb7a2b7 100644 --- a/docs/dev/driving-work-through-the-loop.md +++ b/docs/dev/driving-work-through-the-loop.md @@ -88,7 +88,14 @@ meanwhile is refused with its current state rather than overwritten, and a node For a branch that already has a pull request, name its number for the 422. 2. **Read the review**: `workflow.run.detail` on the child review run gives the verdict and every finding with file and line. `approve` means the publication pushed on its own; - anything else leaves it paused at `approve-push`, and the next round is the fix. + anything else leaves it paused at `approve-push`, as before. Whether the findings take a + fix round is the publication's own decision (Plan 315 D22): `decide-findings` judges each + finding on whether it names a defect its evidence shows, `decide-after-review` chooses + among `fix_before_merge`, `publish_as_is` and `ask` over those that passed, and a fix + round — or an answer below the bar — commissions the fix with those findings as the + brief and publishes it onto the branch once this run's push has gone (`fix-findings`, + `republish-fix`). The run's `afterReview` says which; `ask` is the operator's to + answer, and a run with no decision model leaves the round to the conversation. 3. **The publication runs the folder's checks.** `hb_aflow` declares `node scripts/verify-commit.mjs` (`aflow harness checks hb_aflow -- node scripts/verify-commit.mjs`), and Local Publish runs it after the commit and before the @@ -115,13 +122,19 @@ scripts/verify-commit.mjs`), and Local Publish runs it after the commit and befo touched file by path imports nothing from it, so every `.mjs` or `.ts` test of those workspaces whose source names a touched file's repository path, whole (`scripts/verify-commit.mjs`, not `verify-commit`), in a string literal runs too, counted - and named in the same line as the tests imports reach. A failure fails - the publication with the end of what it printed and nothing pushed: read it on the - `check-commit` task, and commission the fix onto the branch. A run whose only failures - were tests running out of their time names them in `testTimeouts`, and the publication - runs the checks once more itself as `recheck-commit`; no retry from the conversation is - needed, and a failure there is read on that task. The pull request's CI - remains the proof. By hand, from a checkout, it measures `HEAD` against `origin/main`. + and named in the same line as the tests imports reach. The script names the step + that failed in its report, and the check's summary carries it. What follows a failure + is the publication's own decision (Plan 315 D22), read on `decide-after-check`: a + failure of time alone runs the checks once more as `recheck-commit`; a failure the + change caused is commissioned as `fix-checks`, briefed with the check's own summary, and + published onto the branch as `republish-fix`, whose pull request is the run's; and a + machine that cannot run the checks as declared — or an answer below the bar, or no + decision model — fails the publication at `checks-failed` with the end of what they + printed and nothing pushed, which is the conversation's to take up. No retry from the + conversation is needed for the first two, and a failure of either is read on its task. + Fix rounds stop after two: the run the second publishes is too deep to start a + commission of its own. The pull request's CI remains the proof. By hand, from a + checkout, it measures `HEAD` against `origin/main`. 4. **The operator merges.** A merge lands on the dev stack only when the stream that runs it merges `origin/main` into `live` (below). diff --git a/docs/plans/315-local-first-run-ergonomics.md b/docs/plans/315-local-first-run-ergonomics.md index e8227344..a332a74f 100644 --- a/docs/plans/315-local-first-run-ergonomics.md +++ b/docs/plans/315-local-first-run-ergonomics.md @@ -58,6 +58,8 @@ The loop delivered by the local operating model works. Reaching it from a first **D21 — The appliance survives sleep.** The machine the loop runs on is a laptop, and laptops sleep: one night it went into idle sleep with a commission in flight, briefs were posted in the dark-wake windows macOS opens while it sleeps, and two Helmsman conversations died (F109). Two things failed, and each has its own answer. The first is the machine sleeping under running work, and the answer is **the hold**: while the host executor has a step running it holds a sleep assertion — a child `caffeinate` on macOS, `systemd-inhibit` on Linux — taken when the first step starts and released when the last one settles, never between two that overlap, and ended with the executor. Whether to hold is the operator's, a machine-wide field of the policy beside the harnesses it protects: `on-ac`, the default, holds while the machine is on power, because a laptop held awake on battery is one an operator closed expecting it to stop; `always` holds on battery too; `never` holds nothing. The second is what the platform made of the sleep. Every executor's heartbeat lapses while its machine sleeps, so on waking every executor is missing until its first tick, and a missing executor failed work terminally on every dispatch path — a synthetic FAILED whose classification was dropped on purpose, a session failed outright by a timer, a workflow task given the raw message as its failure reason. The answer is **the wait**: a step or operation task whose executor is missing is parked on its shard timer and dispatched when a look finds the executor back, the looks growing apart up to the heartbeat's own lifetime, as many as span ten minutes on a machine that stays awake, after which it fails as what it is — a transient, retryable outage, its classification kept. A parked step is visible as waiting from the moment it parks, naming the executor it waits for in the run view and the chat, and one whose executor this orchestrator has never seen since it started fails as `configuration`, not retryable, because that executor was never started rather than asleep. The wait is counted in looks rather than in time because a sleeping machine takes none, so a wait it sleeps through is not spent on waking, and a look that comes a heartbeat's lifetime or more late is that wake, counts for nothing and is followed by one more. The classification was dropped to stop one outage entering every step behind it into the retry budget at once; the budget of looks and the growing gap prevent that now, since nothing enters a retry budget while it waits, a lane that returns costs no retry at all, and one that does not costs each step one failure, once its looks are spent. A waiting step is its own completion path, not a stalled pickup: orphan recovery and the stall watchdog leave it alone while its timer lives, however long that is, and arm a timer that went missing again rather than fail the step. The host lane is no exception, though its executor is the operator's to start: a sleeping machine is exactly the case where the executor comes back. **The cold restart** — a sleep long enough that the stack itself comes back cold, its orchestrator restarted and the conversations it held to be recovered — is not in this decision. Whether it happens and what recovery costs are to be measured first, and it is a slice of its own. +**D22 — Local Publish decides its own next step after a failed check and after a review's findings, and nothing it decides clears a push.** Both choices were a Helmsman turn: a failed check woke the conversation to choose between running the checks again, commissioning a fix and publishing again, or asking the operator; a review with findings woke it to choose whether a fix round was worth it and to write the brief. Neither choice writes anything a person reads, and both are asked many times with the same questions, which is what a decision step is for (Plan 316 D2). So the skill asks them itself through `ai.decision.decide`, the questions written once in `publishLocalChangesDecisions.ts`. After a failed check: one `choice` among `recheck_on_timeout`, `fix_commission` and `operator_install_mismatch` over the check's summary, tail, failing step and timings. A recheck runs `recheck-commit`, which replaces the old rule that only `testTimeouts` earned one. A fix starts Commission Change on the commit, briefed with a fixed opening and the check's own summary — which names the step the checks report failing (`failingGuard`, from the check report `scripts/verify-commit.mjs` writes) and carries the tail — then publishes the fix onto the branch as a publication of its own. A mismatch, an answer below the bar, or no decision at all fails the run at `checks-failed` (`workflow.run.fail`, a step-only primitive that fails its task with the reason it is handed), so a failed check wakes the conversation exactly as it did before. After a review with findings: one `yes_no` per finding — "names a defect in the change's behaviour, established by evidence shown" — through the operation's new `each`, then, over the findings that passed, one `choice` among `fix_before_merge`, `publish_as_is` and `ask` at a high bar. A fix, or an answer below the bar, commissions a fix briefed with the passing findings as the review returned them and publishes it once this run's push has gone or was declined to go; `ask` and `publish_as_is` change nothing and are recorded as `afterReview`. No prose author writes either brief. The push gate, the scan and the posture are untouched (Plan 316 D9): a decision reads text that can steer it, so it is never the only gate on anything that leaves the machine, and the tests prove the push and its approval go exactly as they did without it. A space with no decision model gets the old behaviour on both paths. Fix rounds end at two: `MAX_WORKFLOW_RUN_DEPTH` is three, so a republication can review itself, and the run a second fix publishes cannot start a commission. The bars are provisional until Plan 316 §7's measurement on this stream's own history (`yarn decisions:measure-loop`) sets them; until then they sit high, so an unsure answer takes the old path or, after a review, the fix. + ## 3. Measurement **First D13 reading** (2026-09-30, Full Circle): asked for a review "without sitting on it", Helmsman chose `wait: 'none'` unprompted, the start returned in 151 ms, the session idled at its prompt, and the review's completion woke it; it reported the verdict with no operator message in between. diff --git a/docs/plans/316-workflows-decide-without-a-language-model.md b/docs/plans/316-workflows-decide-without-a-language-model.md index 5de30b01..6a3ac1e2 100644 --- a/docs/plans/316-workflows-decide-without-a-language-model.md +++ b/docs/plans/316-workflows-decide-without-a-language-model.md @@ -1,6 +1,6 @@ # Plan 316 — Workflows decide without a language model: a typed decision step -**Status:** 🔨 P0–P4 built and proven live; P5 and draft-level editing open · **First backend:** TypeSafe AI's Jev (a "System One" decision model) +**Status:** 🔨 P0–P4 built and proven live; P5 and draft-level editing open · **First backend:** TypeSafe AI's Jev (a "System One" decision model) · **First production consumer:** Publish Local Changes, which decides what follows a failed check and whether a review's findings take a fix round (Plan 315 D22); its bars wait on §7's measurement, which has not run yet ## 1. Problem @@ -42,6 +42,7 @@ Input: - `{ type: 'choice', instructions?, options: Record, minConfidence? }`, 2–255 options - `{ type: 'score', instructions?, levels: (description | null)[], minConfidence? }`, 2–10 levels, index 0 lowest - `{ type: 'yes_no', instructions?, criteria?: { true?, false? }, minConfidence? }` +- `each` — optional `{ items, question }`: one `yes_no` asked of every item of a list (at most 50), for a list whose length is known only when the step runs — a review's findings. Each item travels in its own question's instructions beside the claim, all in the same single pass. `questions` may then be absent; one of the two is required. An empty list asks nothing and needs no model. - `model` — optional catalog reference; the catalog's default decision model when absent. Output: @@ -50,6 +51,7 @@ Output: - choice: `{ type, value: label, confidence, probabilities: Record, decided }` - score: `{ type, value: expected score, confidence, probabilities: Record, decided }` - yes_no: `{ type, value: p ≥ 0.5, probability: p, confidence: max(p, 1 − p), decided }` +- `each` — where asked: every item's `yes_no` answer in order, `selected` (the items answered true and decided), `selectedCount`, `undecidedCount`, and `selectedJson`, the selected items as text a later step can put in a brief. - `model`, `usage { inputTokens, outputTokens }`, `latencyMs`. For a `yes_no`, `confidence` is derived as the probability of the more likely outcome, so one threshold reads the same way on every question type. @@ -110,3 +112,35 @@ The `classifier` rails execute through the operation when a rail opts in. Exit: - A draft whose route names a label the question does not offer is refused with a diagnostic naming the label. - A fork reading `tasks.triage.output.answers.team.valeu` is refused at validation, not skipped at run time. - The operation never appears in an agent's tool list, and does appear in authoring search. + +## 7. Measurement — the loop's own decisions + +**Status: built, not yet run.** No numbers are recorded here until it has run on the operator's stack. The checkout that built it had no running stack holding this stream's runs, no GitHub credential for the Actions logs of PRs #91–#124, and no TypeSafe or OpenAI key. The script's labelling, scoring and report are tested on planted records (`scripts/loop-decision-set.test.mjs`). + +**What it measures.** The two choices Publish Local Changes now makes for itself (Plan 315 D22), plus the CI remedy choice Plan 321 makes by hand, against what was actually decided in this stream. Four questions, each the one the skill asks, imported from `publishLocalChangesDecisions.ts`, so the measurement measures what ships: + +- **After a failed check**: the check's account → what the next publication of the branch did. A change appended means `fix_commission`; the same head published as it stands means `recheck_on_timeout`. +- **Each review finding**: the finding → whether a later publication carried it `closed` (a defect, fixed) or `open` (left — accepted or dismissed, which the records do not tell apart). +- **After a review with findings**: the findings → `fix_before_merge` where a later change closed any of them, `publish_as_is` where they were carried on all open. +- **A failing CI job of PRs #91–#124**: the last 300 lines of the job's log (Plan 321 §P0's bound) → `rerun` where another attempt of the same run followed on the same commit, `merge_round` for a merge commit next, `resolutions_bump` for a next commit touching only `package.json` and `yarn.lock` under `resolutions`, and `code_fix` otherwise. + +A case nothing followed is left unlabelled, not guessed. `operator_install_mismatch` and `ask` are never inferred; they come only from the operator's labels file, which overrides any inference. The report counts how many labels came from the operator and how many were inferred. + +**Deciders.** Jev through `AIClient.decide`, the call the decision operation makes. Then a critique-then-verdict `generate_json` call — a critique, a verdict among the same options and a stated probability that the verdict is right — on gpt-6-luna, and the same call on claude-opus-5-5 as the agent baseline. A language model's stated probability is not calibrated; it is reported as what it is. + +**Recorded per decider and question**: agreement with the operator's decision among the cases it decided; abstention at the bar chosen for it (the lowest bar in the 0.5–0.95 sweep at which agreement reaches the target, 90% by default); agreement over all cases with an abstention counted as not agreeing; errors apart from both; p50 and p95 latency; and cost per decision from the catalog's prices. Jev's whole sweep goes beside the table. + +**Why a script and not the eval plane's dataset (Plan 269).** A golden case there is a run's input and output, labelled by the operator against a skill's suite. These cases are decisions spread across runs and across GitHub — a check's account against the next publication's input, a log against the next commit — and none of them is a run of one skill. So it is a script, as Plan 323 P0 was. + +**Run it**, on the stack that holds the stream, with `AFLOW_API_KEY` set to an API key of the space's tenant and `gh` signed in: + +``` +yarn decisions:measure-loop collect --space --repo aflowai/aflow --prs 91-124 +yarn decisions:measure-loop measure [--labels ] [--target 0.9] +``` + +The tables in `.debug/decisions/report.md` go here, and the bars chosen for Jev replace the provisional `AFTER_CHECK_MIN_CONFIDENCE` (0.8), `FINDING_MIN_CONFIDENCE` (0.8) and `AFTER_REVIEW_MIN_CONFIDENCE` (0.9). Where Jev does not reach the target on a question at any bar, that question's decision step does not belong in Publish Local Changes, and it should be taken out again. + +| Question | Decider | minConfidence | Agreement (decided) | Abstention | p50 ms | Cost per decision | +| -------- | ----------- | ------------- | ------------------- | ---------- | ------ | ----------------- | +| — | not yet run | — | — | — | — | — | diff --git a/package.json b/package.json index 0ab7cc54..172d70cf 100644 --- a/package.json +++ b/package.json @@ -25,6 +25,7 @@ "db:migrate-tenants": "NODE_OPTIONS='--conditions=ts-source' node scripts/with-stack-env.mjs npx tsx scripts/migrate-tenants.ts", "db:seed": "NODE_OPTIONS='--conditions=ts-source' node scripts/with-stack-env.mjs tsx scripts/seed-flows.ts", "db:seed:cybernetic": "NODE_OPTIONS='--conditions=ts-source' node scripts/with-stack-env.mjs tsx scripts/dev/seed-cybernetic-space.ts", + "decisions:measure-loop": "NODE_OPTIONS='--conditions=ts-source' node scripts/with-stack-env.mjs tsx scripts/decisions-measure-loop.ts", "dev": "node scripts/dev.mjs", "start": "node scripts/start.mjs", "dev:core": "node scripts/dev.mjs --profile core", diff --git a/packages/ai-client/src/decision.test.ts b/packages/ai-client/src/decision.test.ts index 5c102597..bcbcef97 100644 --- a/packages/ai-client/src/decision.test.ts +++ b/packages/ai-client/src/decision.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from 'vitest'; import type { DecisionQuestions } from '@aflow/schemas'; -import { resolveDecisionAnswers } from './decision.js'; +import { + decisionRequestQuestions, + resolveDecisionAnswers, + splitDecisionAnswers, +} from './decision.js'; import { AIClientError } from './errors.js'; const questions: DecisionQuestions = { @@ -70,3 +74,64 @@ describe('resolveDecisionAnswers', () => { ).toThrow(/yes_no question and got a score answer/); }); }); + +describe('a question asked of each item', () => { + const each = { + items: [{ claim: 'leaks a handle' }, { claim: 'a name could be better' }, { claim: 'unsure' }], + question: { + type: 'yes_no' as const, + instructions: 'Names a defect, established by evidence shown', + minConfidence: 0.8, + }, + }; + const named: DecisionQuestions = { + round: { type: 'choice', options: { fix: null, ship: null } }, + }; + + it('asks it once per item, each item beside the claim, alongside the named questions', () => { + const asked = decisionRequestQuestions({ questions: named, each }); + expect(Object.keys(asked)).toEqual(['round', 'each_0', 'each_1', 'each_2']); + expect(asked['each_1']).toEqual({ + type: 'yes_no', + instructions: { + claim: 'Names a defect, established by evidence shown', + about: 'The item below, one of a list the state describes.', + item: { claim: 'a name could be better' }, + }, + minConfidence: 0.8, + }); + }); + + it('selects only the items answered true and decided — never one the model was unsure of', () => { + const input = { questions: named, each }; + const asked = decisionRequestQuestions(input); + const resolved = resolveDecisionAnswers(asked, { + round: { type: 'choice', choice: 'fix', confidence: 0.9, probabilities: {} }, + each_0: { type: 'yes_no', probability: 0.95 }, + each_1: { type: 'yes_no', probability: 0.05 }, + each_2: { type: 'yes_no', probability: 0.7 }, + }); + const { answers, each: out } = splitDecisionAnswers(input, resolved); + expect(Object.keys(answers)).toEqual(['round']); + expect(out?.answers.map((a) => [a.value, a.decided])).toEqual([ + [true, true], + [false, true], + [true, false], + ]); + expect(out?.selected).toEqual([{ claim: 'leaks a handle' }]); + expect(out?.selectedCount).toBe(1); + expect(out?.undecidedCount).toBe(1); + expect(JSON.parse(out?.selectedJson ?? '')).toEqual([{ claim: 'leaks a handle' }]); + }); + + it('reports none selected as an empty list in both forms', () => { + const input = { each: { ...each, items: [{ claim: 'x' }] } }; + const asked = decisionRequestQuestions(input); + const { each: out } = splitDecisionAnswers( + input, + resolveDecisionAnswers(asked, { each_0: { type: 'yes_no', probability: 0.01 } }), + ); + expect(out?.selected).toEqual([]); + expect(out?.selectedJson).toBe('[]'); + }); +}); diff --git a/packages/ai-client/src/decision.ts b/packages/ai-client/src/decision.ts index a5d7d369..49eb08a0 100644 --- a/packages/ai-client/src/decision.ts +++ b/packages/ai-client/src/decision.ts @@ -1,4 +1,12 @@ -import type { DecisionAnswer, DecisionQuestions } from '@aflow/schemas'; +import { + decisionEachQuestionName, + type DecisionAnswer, + type DecisionEach, + type DecisionEachOutput, + type DecisionEntry, + type DecisionQuestions, + type DecisionYesNoAnswer, +} from '@aflow/schemas'; import type { ProviderDecisionAnswer } from './types.js'; import { AIClientError } from './errors.js'; @@ -71,6 +79,70 @@ export function resolveDecisionAnswers( return resolved; } +/** + * Every question a request asks, as one record the provider answers in a + * single pass: the named questions, then `each`'s question once per item. An + * item travels inside its own question's instructions, beside the claim, so + * every answer is about exactly one item while the state stays shared. + */ +export function decisionRequestQuestions(input: { + questions?: DecisionQuestions | undefined; + each?: DecisionEach | undefined; +}): DecisionQuestions { + const questions: DecisionQuestions = { ...(input.questions ?? {}) }; + if (input.each === undefined) return questions; + const { question, items } = input.each; + items.forEach((item, index) => { + questions[decisionEachQuestionName(index)] = { + ...question, + instructions: { + ...(question.instructions !== undefined ? { claim: question.instructions } : {}), + about: 'The item below, one of a list the state describes.', + item: item as DecisionEntry, + }, + }; + }); + return questions; +} + +/** + * The resolved answers split back into the named questions' and `each`'s: + * an item is selected only where its answer is true and decided, so an item + * the model was unsure of never passes as though it had been judged. + */ +export function splitDecisionAnswers( + input: { questions?: DecisionQuestions | undefined; each?: DecisionEach | undefined }, + resolved: Readonly>, +): { answers: Record; each?: DecisionEachOutput } { + const answers: Record = {}; + for (const name of Object.keys(input.questions ?? {})) { + const answer = resolved[name]; + if (answer !== undefined) answers[name] = answer; + } + if (input.each === undefined) return { answers }; + const eachAnswers: DecisionYesNoAnswer[] = input.each.items.map((_, index) => { + const answer = resolved[decisionEachQuestionName(index)]; + if (answer?.type !== 'yes_no') { + throw mismatch(`no yes_no answer was returned for item ${String(index)}`); + } + return answer; + }); + const selected = input.each.items.filter((_, index) => { + const answer = eachAnswers[index]; + return answer !== undefined && answer.value && answer.decided; + }); + return { + answers, + each: { + answers: eachAnswers, + selected, + selectedCount: selected.length, + undecidedCount: eachAnswers.filter((answer) => !answer.decided).length, + selectedJson: JSON.stringify(selected, null, 2), + }, + }; +} + function mismatch(detail: string): AIClientError { return new AIClientError( `The decision model's answer does not fit the question: ${detail}`, diff --git a/packages/ai-client/src/index.ts b/packages/ai-client/src/index.ts index aea27d4e..f2b9f520 100644 --- a/packages/ai-client/src/index.ts +++ b/packages/ai-client/src/index.ts @@ -180,7 +180,11 @@ export type { FireworksConfig } from './providers/fireworks.js'; export { createRunwareAdapter, runwareTaskUuid } from './providers/runware.js'; // Decisions -export { resolveDecisionAnswers } from './decision.js'; +export { + decisionRequestQuestions, + resolveDecisionAnswers, + splitDecisionAnswers, +} from './decision.js'; // Plan 323: the answer floor prompt budgeting and a local runtime's sizing share export { MIN_ANSWER_TOKENS } from './providers/ollamaPromptSize.js'; diff --git a/packages/platform-artifacts/src/index.ts b/packages/platform-artifacts/src/index.ts index e0c065ac..8c74c734 100644 --- a/packages/platform-artifacts/src/index.ts +++ b/packages/platform-artifacts/src/index.ts @@ -48,6 +48,16 @@ export type { HelmsmanSurface } from './helmsmanSurface.js'; export { PLATFORM_SKILL_BUNDLES, ALL_PLATFORM_WORKFLOWS } from './skillBundles.js'; export { SKILL_CATALOG, getSkillCatalogEntry, listSkillCatalog } from './skillCatalog.js'; +// The questions Publish Local Changes asks, for the measurement that sets their bars +export { + AFTER_CHECK_QUESTION, + AFTER_CHECK_QUESTION_NAME, + AFTER_CHECK_ROUTES, + AFTER_REVIEW_QUESTION, + AFTER_REVIEW_QUESTION_NAME, + AFTER_REVIEW_ROUTES, + FINDING_QUESTION, +} from './skillCatalog/publishLocalChangesDecisions.js'; export { CONNECTOR_CATALOG, diff --git a/packages/platform-artifacts/src/skillCatalog/publishLocalChanges.test.ts b/packages/platform-artifacts/src/skillCatalog/publishLocalChanges.test.ts index 8959481c..0e982ddb 100644 --- a/packages/platform-artifacts/src/skillCatalog/publishLocalChanges.test.ts +++ b/packages/platform-artifacts/src/skillCatalog/publishLocalChanges.test.ts @@ -37,6 +37,15 @@ import { toJsonSchemaSync, } from '@aflow/schemas'; import { PUBLISH_LOCAL_CHANGES } from './publishLocalChanges.js'; +import { + AFTER_CHECK_QUESTION, + AFTER_REVIEW_QUESTION, + FINDING_QUESTION, + FIX_CHECKS_BRIEF_LEAD, + FIX_FINDINGS_BRIEF_LEAD, + type AfterCheckRoute, + type AfterReviewRoute, +} from './publishLocalChangesDecisions.js'; import { REVIEW_LOCAL_CHANGES } from './reviewLocalChanges.js'; const wf = PUBLISH_LOCAL_CHANGES.bundle.workflow; @@ -133,64 +142,76 @@ describe('Publish Local Changes — the patch becomes a branch, then a pull requ expect(validity.status).toBe('valid'); }); - it('is thirteen tasks — commit, check and its one rerun, scan, what decides the approval, the pull request composed, approve, push, find and rewrite or open the pull request', () => { + it('is twenty tasks — commit, check, what follows a failed check, scan, what decides the approval, the review and what follows its findings, the pull request composed, approve, push, the pull request found, rewritten or opened, and a fix round and its publication', () => { expect(wf.tasks.map((t) => t.taskId)).toEqual([ 'commit', 'check-commit', + 'decide-after-check', + 'checks-failed', 'recheck-commit', 'scan-commit', 'read-repository', 'read-push-approval', 'review-commit', + 'decide-findings', + 'decide-after-review', 'describe-pr', 'approve-push', 'push', 'find-pr', 'update-pr', 'open-pr', + 'fix-checks', + 'fix-findings', + 'republish-fix', ]); - expect(wf.tasks.map((t) => t.type)).toEqual([ - 'operation', - 'operation', - 'operation', - 'operation', - 'operation', - 'operation', - 'operation', - 'operation', - 'human', - 'operation', - 'operation', - 'operation', - 'operation', - ]); - expect(wf.tasks.map((t) => t.dependsOn ?? [])).toEqual([ - [], - ['commit'], - ['commit', 'check-commit'], - ['commit', 'check-commit', 'recheck-commit'], - ['commit'], - ['commit', 'scan-commit'], - ['read-push-approval', 'scan-commit'], - ['commit', 'check-commit', 'recheck-commit', 'scan-commit', 'review-commit'], - ['read-repository', 'scan-commit', 'review-commit'], - ['approve-push', 'scan-commit', 'check-commit', 'recheck-commit', 'describe-pr'], - ['push'], - ['find-pr', 'describe-pr'], - ['find-pr', 'describe-pr'], + expect(wf.tasks.filter((t) => t.type !== 'operation').map((t) => t.taskId)).toEqual([ + 'approve-push', ]); - expect(taskOrThrow('describe-pr').operation).toBe('host.commit.describe'); - expect(taskOrThrow('update-pr').operation).toBe('api.http.call'); - expect(taskOrThrow('commit').operation).toBe('host.file.patch'); - expect(taskOrThrow('check-commit').operation).toBe('host.commit.check'); - expect(taskOrThrow('recheck-commit').operation).toBe('host.commit.check'); - expect(taskOrThrow('scan-commit').operation).toBe('host.commit.scan'); - expect(taskOrThrow('read-repository').operation).toBe('api.http.call'); - expect(taskOrThrow('read-push-approval').operation).toBe('host.binding.inspect'); - expect(taskOrThrow('review-commit').operation).toBe('workflow.run.start'); - expect(taskOrThrow('push').operation).toBe('host.process.exec'); - expect(taskOrThrow('find-pr').operation).toBe('api.http.call'); - expect(taskOrThrow('open-pr').operation).toBe('api.http.call'); + expect(Object.fromEntries(wf.tasks.map((t) => [t.taskId, t.dependsOn ?? []]))).toEqual({ + commit: [], + 'check-commit': ['commit'], + 'decide-after-check': ['check-commit'], + 'checks-failed': ['check-commit', 'decide-after-check'], + 'recheck-commit': ['commit', 'check-commit', 'decide-after-check'], + 'scan-commit': ['commit', 'check-commit', 'recheck-commit'], + 'read-repository': ['commit', 'check-commit', 'recheck-commit'], + 'read-push-approval': ['commit', 'scan-commit'], + 'review-commit': ['read-push-approval', 'scan-commit'], + 'decide-findings': ['review-commit'], + 'decide-after-review': ['decide-findings'], + 'describe-pr': ['commit', 'check-commit', 'recheck-commit', 'scan-commit', 'review-commit'], + 'approve-push': ['read-repository', 'scan-commit', 'review-commit'], + push: ['approve-push', 'scan-commit', 'check-commit', 'recheck-commit', 'describe-pr'], + 'find-pr': ['push'], + 'update-pr': ['find-pr', 'describe-pr'], + 'open-pr': ['find-pr', 'describe-pr'], + 'fix-checks': ['commit', 'check-commit', 'decide-after-check'], + 'fix-findings': ['commit', 'decide-findings', 'decide-after-review'], + 'republish-fix': ['fix-checks', 'fix-findings', 'push'], + }); + expect(Object.fromEntries(wf.tasks.map((t) => [t.taskId, t.operation]))).toEqual({ + commit: 'host.file.patch', + 'check-commit': 'host.commit.check', + 'decide-after-check': 'ai.decision.decide', + 'checks-failed': 'workflow.run.fail', + 'recheck-commit': 'host.commit.check', + 'scan-commit': 'host.commit.scan', + 'read-repository': 'api.http.call', + 'read-push-approval': 'host.binding.inspect', + 'review-commit': 'workflow.run.start', + 'decide-findings': 'ai.decision.decide', + 'decide-after-review': 'ai.decision.decide', + 'describe-pr': 'host.commit.describe', + 'approve-push': undefined, + push: 'host.process.exec', + 'find-pr': 'api.http.call', + 'update-pr': 'api.http.call', + 'open-pr': 'api.http.call', + 'fix-checks': 'workflow.run.start', + 'fix-findings': 'workflow.run.start', + 'republish-fix': 'workflow.run.start', + }); }); it('reads the repository on the same GitHub binding the pull request uses, before the approval', () => { @@ -211,8 +232,12 @@ describe('Publish Local Changes — the patch becomes a branch, then a pull requ }); const kinds = Object.values(read.inputBindings ?? {}).map((b) => b.kind); expect(kinds).not.toContain('connection_binding'); + // Only once the checks cleared the commit: a failed check reads nothing more. expect(read.when).toEqual({ - expression: "tasks.commit.output.state == 'applied'", + anyOf: [ + 'tasks.check-commit.output.passed == true', + 'tasks.recheck-commit.output.passed == true', + ], onMissingRef: 'skip', }); }); @@ -629,7 +654,16 @@ describe('Publish Local Changes — the patch becomes a branch, then a pull requ const promoters = wf.tasks.filter((task) => task.promoteOutputs?.some((p) => 'toState' in p && p.toState === RUN_PULL_REQUEST_URL_OUTPUT), ); - expect(promoters.map((task) => task.taskId).sort()).toEqual(['find-pr', 'open-pr']); + // A fix round's publication reports its pull request as this run's: the + // same branch, so the same pull request, and it comes later in the list + // so its value is the one the run keeps. + expect(promoters.map((task) => task.taskId).sort()).toEqual([ + 'find-pr', + 'open-pr', + 'republish-fix', + ]); + const order = wf.tasks.map((task) => task.taskId); + expect(order.indexOf('republish-fix')).toBeGreaterThan(order.indexOf('open-pr')); expect( wf.stateVariables.find((v) => v.variableId === RUN_PULL_REQUEST_URL_OUTPUT)?.sensitive, ).toBe(false); @@ -739,16 +773,104 @@ type Decision = 'approved' | 'declined'; * The review run's ending as the waiter delivers it to the task: the run's * promoted output only when it completed. */ -function reviewEnding(review: Exclude): Record { +function reviewEnding( + review: Exclude, + findings: readonly Finding[] = [], +): Record { const completed = review !== 'failed' && review !== 'cancelled' && review !== 'paused'; return WorkflowRunWakeupEnvelopeSchema.parse({ runId: 'review-run', outcome: completed ? 'completed' : review, waiterId: 'waiter-1', - ...(completed ? { result: { output: { verdict: review, reviewSummary: 'Read.' } } } : {}), + ...(completed + ? { result: { output: { verdict: review, reviewSummary: 'Read.', findings } } } + : {}), + }); +} + +/** A finding as Local Code Review returns it. */ +interface Finding { + severity: 'blocker' | 'major' | 'minor' | 'nit'; + file: string; + line?: number; + claim: string; + evidence: string; +} + +/** + * What a decision model answers, scripted: a choice and its confidence, or + * `unavailable` — no decision model bound, a provider refusing — which fails + * the decision step. + */ +type Choice = { value: Route; confidence: number } | 'unavailable'; + +/** A choice answered as `ai.decision.decide` returns it, decided by the question's own bar. */ +function choiceOutput( + name: string, + minConfidence: number, + choice: { value: string; confidence: number }, +): Record { + return { + answers: { + [name]: { + type: 'choice', + value: choice.value, + confidence: choice.confidence, + probabilities: { [choice.value]: choice.confidence }, + decided: choice.confidence >= minConfidence, + }, + }, + model: 'jev-1.13.0', + usage: { promptTokens: 1, completionTokens: 0, totalTokens: 1 }, + latencyMs: 40, + }; +} + +/** Each finding judged with the probability given for it, as `ai.decision.decide` returns `each`. */ +function judgedOutput(items: readonly unknown[], probabilities: readonly number[]) { + const answers = items.map((_, i) => { + const probability = probabilities[i] ?? 0; + const confidence = Math.max(probability, 1 - probability); + return { + type: 'yes_no' as const, + value: probability >= 0.5, + probability, + confidence, + decided: confidence >= FINDING_QUESTION.minConfidence, + }; + }); + const selected = items.filter((_, i) => answers[i]?.value === true && answers[i].decided); + return { + answers: {}, + each: { + answers, + selected, + selectedCount: selected.length, + undecidedCount: answers.filter((a) => !a.decided).length, + selectedJson: JSON.stringify(selected, null, 2), + }, + model: 'jev-1.13.0', + usage: { promptTokens: 1, completionTokens: 0, totalTokens: 1 }, + latencyMs: 40, + }; +} + +/** The patch a fix round's commission returns, and the base it reports: this run's commit. */ +const FIX_PATCH_REF = 'payload:fix-patch'; + +/** A run a task started and waited on, ending as `output` says, or failing. */ +function runEnding(runId: string, output: Record | 'failed') { + return WorkflowRunWakeupEnvelopeSchema.parse({ + runId, + outcome: output === 'failed' ? 'failed' : 'completed', + waiterId: `waiter-${runId}`, + ...(output === 'failed' ? {} : { result: { output } }), }); } +/** The pull request the publication of a fix ends with. */ +const FIX_PR = { prUrl: 'https://github.com/aflowai/aflow/pull/9', prNumber: 9 }; + type Scan = 'clean' | 'finding' | 'unscanned' | 'allowed'; /** What `host.commit.scan` returns for each scan, before the task's own projection. */ @@ -820,6 +942,8 @@ const CHECK_RECEIPT = { /** The test a check under load names as having run out of its time. */ const TIMED_OUT_TEST = 'apps/aflow-executor-api/src/handlers/api/saveResponse.test.ts › a saveTo download allows a body above the inline default'; +/** The decision after a run lost only to time, sure of it. */ +const RECHECK = { value: 'recheck_on_timeout', confidence: 0.95 } as const; const CHECK_TAIL = 'ok large-files guard (2.0 s)\nFAIL tsc apps/aflow-executor-host/tsconfig.json (41.3 s)\n'; @@ -926,6 +1050,16 @@ interface Scenario { inputs?: Record; /** The composed text holds what looks like a secret, so the composition is refused. */ describeRefused?: boolean; + /** What the decision after a failed check answers; absent is `unavailable`. */ + afterCheck?: Choice; + /** The findings a completed review returns; absent is none. */ + findings?: readonly Finding[]; + /** How likely each finding is judged to name a defect, in order; or `unavailable`. */ + judged?: readonly number[] | 'unavailable'; + /** What the decision on a fix round answers; absent is `unavailable`. */ + afterReview?: Choice; + /** How a fix round's commission ends: with a change, with none, or failed; absent is a change. */ + commission?: 'patch' | 'no-change' | 'failed'; } interface Outcome { @@ -1014,7 +1148,36 @@ function rawOutput( case 'review-commit': if (scenario.review === 'not-started') throw new Error('a review that did not start has no output'); - return reviewEnding(scenario.review); + return reviewEnding(scenario.review, scenario.findings); + case 'decide-after-check': + if (scenario.afterCheck === undefined || scenario.afterCheck === 'unavailable') return {}; + return choiceOutput('next_step', AFTER_CHECK_QUESTION.minConfidence, scenario.afterCheck); + case 'decide-findings': { + const each = input['each'] as { items: unknown[] }; + return judgedOutput( + each.items, + scenario.judged === 'unavailable' ? [] : (scenario.judged ?? []), + ); + } + case 'decide-after-review': + if (scenario.afterReview === undefined || scenario.afterReview === 'unavailable') return {}; + return choiceOutput( + 'after_review', + AFTER_REVIEW_QUESTION.minConfidence, + scenario.afterReview, + ); + case 'fix-checks': + case 'fix-findings': + return runEnding( + `commission-${taskId}`, + scenario.commission === 'failed' + ? 'failed' + : scenario.commission === 'no-change' + ? { baseSha: HEAD } + : { patchRef: FIX_PATCH_REF, baseSha: HEAD }, + ); + case 'republish-fix': + return runEnding(`publication-${taskId}`, FIX_PR); case 'push': return { exitCode: 0 }; case 'find-pr': { @@ -1186,6 +1349,18 @@ function publish(scenario: Scenario): Outcome { if (task.optional === true) failedOptional.add(task.taskId); continue; } + // A decision that could not be made fails its step, as the executor does. + if ( + (task.taskId === 'decide-after-check' && + (scenario.afterCheck ?? 'unavailable') === 'unavailable') || + (task.taskId === 'decide-findings' && scenario.judged === 'unavailable') || + (task.taskId === 'decide-after-review' && + (scenario.afterReview ?? 'unavailable') === 'unavailable') + ) { + statuses.set(task.taskId, 'failed'); + if (task.optional === true) failedOptional.add(task.taskId); + continue; + } if (task.taskId === 'approve-push') { asked = true; if (scenario.decision === 'declined' && scenario.declineAs !== 'answer') { @@ -1219,6 +1394,11 @@ function publish(scenario: Scenario): Outcome { new Set(Object.keys(task.inputBindings ?? {})), ); inputs.set(task.taskId, input); + // `workflow.run.fail` fails its own task whatever it is handed. + if (task.operation === 'workflow.run.fail') { + statuses.set(task.taskId, 'failed'); + continue; + } const raw = rawOutput(task.taskId, scenario, input); if (task.outputProjection !== undefined) { const projected = projectTaskOutput(task.outputProjection, raw, null); @@ -1387,7 +1567,13 @@ describe('Publish Local Changes — the folder decides whether the push asks', ( const projected = projectTaskOutput(review.outputProjection, reviewEnding(outcome), null); expect(projected).toEqual({ ok: true, - value: { verdict: null, reviewSummary: null, outcome, reviewRunId: 'review-run' }, + value: { + verdict: null, + reviewSummary: null, + findings: null, + outcome, + reviewRunId: 'review-run', + }, }); } const approved = projectTaskOutput(review.outputProjection, reviewEnding('approve'), null); @@ -1396,6 +1582,7 @@ describe('Publish Local Changes — the folder decides whether the push asks', ( value: { verdict: 'approve', reviewSummary: 'Read.', + findings: [], outcome: 'completed', reviewRunId: 'review-run', }, @@ -1566,8 +1753,12 @@ describe('Publish Local Changes — the commit is scanned for secrets before any range: `${ORIGIN_BASE}..${HEAD}`, texts: { 'pull request title': 'Fix it' }, }); + // A commit the checks did not clear is never scanned: nothing after them runs. expect(scan.when).toEqual({ - expression: "tasks.commit.output.state == 'applied'", + anyOf: [ + 'tasks.check-commit.output.passed == true', + 'tasks.recheck-commit.output.passed == true', + ], onMissingRef: 'skip', }); }); @@ -1902,8 +2093,8 @@ describe('Publish Local Changes — a branch is published as it stands', () => { expect(outcome.ran).toEqual([ 'commit', 'check-commit', - 'read-repository', 'scan-commit', + 'read-repository', 'read-push-approval', 'review-commit', 'describe-pr', @@ -1987,7 +2178,7 @@ describe("Publish Local Changes — the folder's checks run before anything leav expect(outcome.pushed).toBe(true); }); - it('fails at the check, rather than pausing on a resolution that could clear it', () => { + it('records every ending of the first run, so what follows a failure is decided — and fails at the second', () => { for (const taskId of ['check-commit', 'recheck-commit']) { const check = taskOrThrow(taskId); expect(check.maxAttempts, taskId).toBe(1); @@ -1996,13 +2187,10 @@ describe("Publish Local Changes — the folder's checks run before anything leav } const projects = (taskId: string, ending: Check): boolean => projectTaskOutput(taskOrThrow(taskId).outputProjection ?? {}, checkResult(ending), null).ok; - for (const ending of ['failed', 'timed-out'] as const) { - expect(projects('check-commit', ending), ending).toBe(false); - } - for (const ending of ['passed', 'skipped', 'test-timeouts'] as const) { + for (const ending of ['passed', 'skipped', 'failed', 'timed-out', 'test-timeouts'] as const) { expect(projects('check-commit', ending), ending).toBe(true); } - // The second run is the last: tests running out of time fail it as anything else does. + // The second run is the last: anything but passing fails it, and the run. for (const ending of ['failed', 'timed-out', 'test-timeouts'] as const) { expect(projects('recheck-commit', ending), ending).toBe(false); } @@ -2028,6 +2216,7 @@ describe("Publish Local Changes — the folder's checks run before anything leav review: 'approve', decision: 'approved', check: 'test-timeouts', + afterCheck: RECHECK, }); expect(checkRuns(outcome.ran)).toEqual(['check-commit', 'recheck-commit']); expect(outcome.ran.indexOf('scan-commit')).toBeGreaterThan( @@ -2049,6 +2238,7 @@ describe("Publish Local Changes — the folder's checks run before anything leav review: 'approve', decision: 'approved', check: 'test-timeouts', + afterCheck: RECHECK, }); expect(outcome.statuses.get('check-commit')).toBe('succeeded'); expect(outcome.outputs.get('check-commit')).toMatchObject({ @@ -2063,7 +2253,7 @@ describe("Publish Local Changes — the folder's checks run before anything leav receipt: CHECK_RECEIPT.passed, }); expect(taskOrThrow('recheck-commit').name).toBe( - "Run the folder's checks again, after tests ran out of time", + "Run the folder's checks again, after they ran out of time", ); }); @@ -2074,6 +2264,7 @@ describe("Publish Local Changes — the folder's checks run before anything leav review: 'approve', decision: 'approved', check: 'test-timeouts', + afterCheck: RECHECK, recheck, }); expect(checkRuns(outcome.ran)).toEqual(['check-commit', 'recheck-commit']); @@ -2158,8 +2349,11 @@ describe("Publish Local Changes — the folder's checks run before anything leav it("says in its description that the folder's checks run first, and that the pull request's remain the proof", () => { expect(PUBLISH_LOCAL_CHANGES.description).toContain( - 'run in a checkout of the commit before the scan, and a failure stops the run with nothing scanned, reviewed or pushed', + 'run in a checkout of the commit before the scan, and nothing is scanned, reviewed or pushed until they pass', ); + // What a failed check leads to is the run's own decision, and an unsure one stops it. + expect(PUBLISH_LOCAL_CHANGES.description).toContain('or a stop, as an unsure decision is.'); + expect(PUBLISH_LOCAL_CHANGES.description).toContain('No decision clears or holds a push.'); expect(PUBLISH_LOCAL_CHANGES.description).toContain('the push needs their passing receipt.'); expect(PUBLISH_LOCAL_CHANGES.description).toContain('A folder that declares none runs none.'); expect(PUBLISH_LOCAL_CHANGES.description).toContain( @@ -2274,6 +2468,7 @@ describe('Publish Local Changes — the pull request describes the branch as a w review: 'approve', decision: 'approved', check: 'test-timeouts', + afterCheck: RECHECK, }); const described = describedWith(outcome); expect(described.check?.passed).toBe(false); @@ -2381,3 +2576,386 @@ describe('Publish Local Changes — the pull request describes the branch as a w expect(runInputOrThrow('gaps').schema).toEqual({ ...toJsonSchemaSync(PullRequestGapsSchema) }); }); }); + +// ── What follows a failed check, decided by the skill ────────────────────── + +describe('Publish Local Changes — a failed check is decided on, not handed to a conversation', () => { + /** Everything that only runs on a commit the checks cleared. */ + const CLEARED_ONLY = [ + 'scan-commit', + 'read-repository', + 'read-push-approval', + 'review-commit', + 'describe-pr', + 'approve-push', + 'push', + 'find-pr', + 'open-pr', + ]; + const FIX_ROUND = ['fix-checks', 'republish-fix']; + + function failedWith(afterCheck: Choice, extra: Partial = {}): Outcome { + return publish({ + pushApproval: 'never', + review: 'approve', + decision: 'approved', + check: 'failed', + afterCheck, + inputs: { owner: 'aflowai', repo: 'aflow' }, + ...extra, + }); + } + + it("asks one choice of the three routes, over the checks' own account, and only when they did not pass", () => { + const decide = taskOrThrow('decide-after-check'); + const questions = (decide.inputTemplate as { questions: Record }) + .questions; + expect(Object.keys(questions)).toEqual(['next_step']); + expect(Object.keys(questions['next_step']?.options ?? {})).toEqual([ + 'recheck_on_timeout', + 'fix_commission', + 'operator_install_mismatch', + ]); + expect(decide.when).toEqual({ + expression: 'tasks.check-commit.output.passed == false', + onMissingRef: 'skip', + }); + const outcome = failedWith({ value: 'fix_commission', confidence: 0.95 }); + expect(outcome.inputs.get('decide-after-check')?.['state']).toEqual({ + summary: checkResult('failed')['summary'], + tail: CHECK_TAIL, + failingGuard: null, + timedOut: null, + testTimeouts: null, + }); + for (const check of ['passed', 'skipped'] as const) { + expect(failedWith(RECHECK, { check }).ran, check).not.toContain('decide-after-check'); + } + }); + + it('recheck_on_timeout: runs the checks once more, and pushes only on that run passing', () => { + const passes = failedWith(RECHECK, { check: 'timed-out' }); + expect(passes.ran).toContain('recheck-commit'); + expect(passes.pushed).toBe(true); + expect(passes.inputs.get('push')?.['check']).toEqual({ receipt: CHECK_RECEIPT.passed }); + for (const id of [...FIX_ROUND, 'checks-failed']) expect(passes.ran, id).not.toContain(id); + + const failsAgain = failedWith(RECHECK, { check: 'timed-out', recheck: 'failed' }); + expect(failsAgain.statuses.get('recheck-commit')).toBe('failed'); + for (const id of CLEARED_ONLY) expect(failsAgain.ran, id).not.toContain(id); + }); + + it("fix_commission: commissions the fix on this commit, briefed from the checks' account alone, then publishes the branch again", () => { + const outcome = failedWith({ value: 'fix_commission', confidence: 0.95 }); + expect(outcome.ran).toEqual(expect.arrayContaining(FIX_ROUND)); + // Nothing of this commit leaves the machine from this run: the + // republication checks, scans, reviews and pushes it as any publication does. + for (const id of [...CLEARED_ONLY, 'recheck-commit', 'checks-failed']) { + expect(outcome.ran, id).not.toContain(id); + } + const commission = WorkflowRunStartInputSchema.parse(outcome.inputs.get('fix-checks')); + expect(commission).toMatchObject({ + slug: 'commission-change', + catalogId: 'commission-change', + wait: 'until_complete', + }); + // The brief is the fixed lead and the checks' own summary — which names + // the failing step and carries the tail — and nothing a model wrote. + expect(commission.inputs).toEqual({ + bindingId: 'folder-1', + task: `${FIX_CHECKS_BRIEF_LEAD}${String(checkResult('failed')['summary'])}`, + base: HEAD, + }); + expect(String(commission.inputs?.['task'])).toContain(CHECK_TAIL); + + const republish = WorkflowRunStartInputSchema.parse(outcome.inputs.get('republish-fix')); + expect(republish).toMatchObject({ + slug: 'publish-local-changes', + catalogId: 'publish-local-changes', + wait: 'until_complete', + }); + // Appended to this branch at this commit, which is what the commission was made against. + expect(republish.inputs).toMatchObject({ + bindingId: 'folder-1', + branch: 'aflow/x', + base: 'main', + owner: 'aflowai', + repo: 'aflow', + patchRef: FIX_PATCH_REF, + baseSha: HEAD, + }); + // The commit names the round it came from: its commission's run and this commit. + expect(republish.inputs?.['commitMessage']).toBe( + `Make the folder's checks pass\n\nThe fix commission commission-fix-checks made from what the folder's checks printed when they failed on ${HEAD}.`, + ); + expect(outcome.state['prUrl']).toBe(FIX_PR.prUrl); + }); + + it('names the failing step in the brief where the checks named one', () => { + const summary = 'node scripts/verify-commit.mjs exited 1: the check failed at `eslint`.'; + const brief = substituteTemplateBinds( + taskOrThrow('fix-checks').inputTemplate ?? {}, + { bindingId: 'folder-1', commitSha: HEAD, summary }, + new Set(Object.keys(taskOrThrow('fix-checks').inputBindings ?? {})), + ); + expect((brief['inputs'] as { task: string }).task).toBe(`${FIX_CHECKS_BRIEF_LEAD}${summary}`); + }); + + for (const ending of ['no-change', 'failed'] as const) { + it(`fix_commission: a commission that ${ending === 'failed' ? 'fails' : 'returns no change'} fails the publication, and nothing is published again`, () => { + const outcome = failedWith( + { value: 'fix_commission', confidence: 0.95 }, + { commission: ending }, + ); + expect(outcome.statuses.get('fix-checks')).toBe('failed'); + expect(outcome.ran).not.toContain('republish-fix'); + expect(outcome.pushed).toBe(false); + expect(taskOrThrow('fix-checks').failureInstruction).toContain( + 'Nothing was scanned, reviewed or pushed', + ); + }); + } + + const STOPS: Array<[string, Choice]> = [ + ['operator_install_mismatch', { value: 'operator_install_mismatch', confidence: 0.95 }], + ['an undecided fix_commission', { value: 'fix_commission', confidence: 0.79 }], + ['an undecided recheck_on_timeout', { value: 'recheck_on_timeout', confidence: 0.5 }], + ['no decision at all', 'unavailable'], + ]; + for (const [label, afterCheck] of STOPS) { + it(`${label}: fails the publication as a failed check always did, with the checks' account as the reason`, () => { + const outcome = failedWith(afterCheck); + expect(outcome.statuses.get('checks-failed')).toBe('failed'); + expect(outcome.inputs.get('checks-failed')).toEqual({ + reason: checkResult('failed')['summary'], + }); + for (const id of [...CLEARED_ONLY, ...FIX_ROUND, 'recheck-commit']) { + expect(outcome.ran, id).not.toContain(id); + } + expect(outcome.pushed).toBe(false); + expect(outcome.state['prUrl']).toBeUndefined(); + }); + } + + it('fails with the same instruction a failed check always carried', () => { + const stop = taskOrThrow('checks-failed'); + expect(stop.failureInstruction).toBe(taskOrThrow('check-commit').failureInstruction); + expect(stop.optional).not.toBe(true); + // The decision is optional so that its own failure is a route, not the run's end. + expect(taskOrThrow('decide-after-check').optional).toBe(true); + }); + + it('abstains below the bar the question sets, whatever it chose', () => { + expect(AFTER_CHECK_QUESTION.minConfidence).toBeGreaterThanOrEqual(0.8); + const at = failedWith({ + value: 'fix_commission', + confidence: AFTER_CHECK_QUESTION.minConfidence, + }); + expect(at.ran).toContain('fix-checks'); + const below = failedWith({ + value: 'fix_commission', + confidence: AFTER_CHECK_QUESTION.minConfidence - 0.01, + }); + expect(below.ran).not.toContain('fix-checks'); + expect(below.ran).toContain('checks-failed'); + }); +}); + +// ── Whether a review's findings take a fix round, decided by the skill ───── + +describe("Publish Local Changes — a review's findings are judged one at a time, and a fix round decided", () => { + const LEAK: Finding = { + severity: 'major', + file: 'src/cache.ts', + line: 40, + claim: 'The cache is never invalidated after a write.', + evidence: + 'put() writes the store and leaves the map entry; the test reads only before the write.', + }; + const NAME: Finding = { + severity: 'nit', + file: 'src/cache.ts', + claim: 'The name says what it was.', + evidence: 'cacheOld', + }; + const RACE: Finding = { + severity: 'blocker', + file: 'src/lock.ts', + line: 12, + claim: 'Two writers can both take the lock.', + evidence: 'The check and the set are two calls.', + }; + const FINDINGS = [LEAK, NAME, RACE]; + + function reviewedWith(extra: Partial): Outcome { + return publish({ + pushApproval: 'unless-unreviewed', + review: 'request_changes', + decision: 'approved', + findings: FINDINGS, + inputs: { owner: 'aflowai', repo: 'aflow' }, + ...extra, + }); + } + + /** Whether the push and its approval went as they would with no decision at all. */ + function gateAsBefore(outcome: Outcome, scenario: Partial): void { + const without = publish({ + pushApproval: 'unless-unreviewed', + review: 'request_changes', + decision: 'approved', + inputs: { owner: 'aflowai', repo: 'aflow' }, + ...scenario, + findings: [], + }); + expect(outcome.asked).toBe(without.asked); + expect(outcome.pushed).toBe(without.pushed); + } + + it('asks, of each finding, whether it names a defect established by the evidence shown', () => { + const outcome = reviewedWith({ + judged: [0.95, 0.1, 0.9], + afterReview: { value: 'publish_as_is', confidence: 0.95 }, + }); + const asked = outcome.inputs.get('decide-findings') as { + each: { items: unknown[]; question: unknown }; + }; + expect(asked.each.items).toEqual(FINDINGS); + expect(asked.each.question).toEqual(FINDING_QUESTION); + expect(FINDING_QUESTION.instructions).toBe( + "The finding names a defect in the change's behaviour, established by evidence shown.", + ); + }); + + it('decides on a fix round only over the findings that passed, and only where one did', () => { + const some = reviewedWith({ + judged: [0.95, 0.1, 0.9], + afterReview: { value: 'publish_as_is', confidence: 0.95 }, + }); + expect( + (some.inputs.get('decide-after-review')?.['state'] as { findings: unknown }).findings, + ).toEqual([LEAK, RACE]); + // Dismissed, or judged with too little confidence to count: no round is weighed. + const none = reviewedWith({ judged: [0.05, 0.1, 0.6] }); + expect(none.ran).toContain('decide-findings'); + expect(none.ran).not.toContain('decide-after-review'); + expect(none.ran).not.toContain('fix-findings'); + }); + + it('fix_before_merge: briefs the commission with the findings that passed, in order, and nothing else', () => { + const outcome = reviewedWith({ + judged: [0.95, 0.1, 0.9], + afterReview: { value: 'fix_before_merge', confidence: 0.95 }, + }); + const commission = WorkflowRunStartInputSchema.parse(outcome.inputs.get('fix-findings')); + expect(commission).toMatchObject({ slug: 'commission-change', catalogId: 'commission-change' }); + expect(commission.inputs).toEqual({ + bindingId: 'folder-1', + task: `${FIX_FINDINGS_BRIEF_LEAD}${JSON.stringify([LEAK, RACE], null, 2)}`, + base: HEAD, + }); + expect(String(commission.inputs?.['task'])).not.toContain(NAME.claim); + gateAsBefore(outcome, {}); + }); + + it("fix_before_merge: publishes the fix only after this run's push, appended at this commit", () => { + const outcome = reviewedWith({ + judged: [0.95, 0.1, 0.9], + afterReview: { value: 'fix_before_merge', confidence: 0.95 }, + }); + expect(outcome.pushed).toBe(true); + expect(outcome.ran.indexOf('republish-fix')).toBeGreaterThan(outcome.ran.indexOf('push')); + const republish = WorkflowRunStartInputSchema.parse(outcome.inputs.get('republish-fix')); + expect(republish).toMatchObject({ + slug: 'publish-local-changes', + catalogId: 'publish-local-changes', + }); + expect(republish.inputs).toMatchObject({ + branch: 'aflow/x', + patchRef: FIX_PATCH_REF, + baseSha: HEAD, + }); + expect(String(republish.inputs?.['commitMessage'])).toMatch( + /^Fix the review's findings\n\nThe fix commission commission-fix-findings made /, + ); + expect(outcome.state['prUrl']).toBe(FIX_PR.prUrl); + }); + + it('fix_before_merge: publishes nothing again where the operator declined this push', () => { + const outcome = reviewedWith({ + decision: 'declined', + judged: [0.95, 0.1, 0.9], + afterReview: { value: 'fix_before_merge', confidence: 0.95 }, + }); + expect(outcome.pushed).toBe(false); + expect(outcome.ran).not.toContain('republish-fix'); + }); + + it('undecided: takes the fix round, briefed with the same findings', () => { + const outcome = reviewedWith({ + judged: [0.95, 0.1, 0.9], + afterReview: { + value: 'publish_as_is', + confidence: AFTER_REVIEW_QUESTION.minConfidence - 0.01, + }, + }); + expect(outcome.ran).toContain('fix-findings'); + expect( + String((outcome.inputs.get('fix-findings')?.['inputs'] as { task: string }).task), + ).toContain(LEAK.claim); + gateAsBefore(outcome, {}); + }); + + for (const value of ['publish_as_is', 'ask'] as const) { + it(`${value}: takes no fix round, records the decision, and leaves the push as it was`, () => { + const outcome = reviewedWith({ + judged: [0.95, 0.1, 0.9], + afterReview: { value, confidence: 0.95 }, + }); + expect(outcome.ran).not.toContain('fix-findings'); + expect(outcome.ran).not.toContain('republish-fix'); + expect(outcome.state['afterReview']).toBe(value); + gateAsBefore(outcome, {}); + }); + } + + it('sets its bar high: only a confident choice keeps a branch from its fix round', () => { + expect(AFTER_REVIEW_QUESTION.minConfidence).toBeGreaterThanOrEqual(0.9); + }); + + for (const [label, extra] of [ + ['the findings cannot be judged', { judged: 'unavailable' }], + ['the round cannot be decided', { judged: [0.95, 0.1, 0.9], afterReview: 'unavailable' }], + ] as const) { + it(`no fix round where ${label}: the run goes on as before, the findings left to whoever reads them`, () => { + const outcome = reviewedWith(extra as Partial); + expect(outcome.ran).not.toContain('fix-findings'); + gateAsBefore(outcome, {}); + }); + } + + it('judges nothing where the review returned no findings or did not finish', () => { + for (const review of ['approve', 'failed', 'not-started'] as const) { + const outcome = reviewedWith({ review, findings: review === 'approve' ? [] : FINDINGS }); + expect(outcome.ran, review).not.toContain('decide-findings'); + } + }); + + it('leaves the push gate, the scan and the posture exactly as they were', () => { + // 316 D9: a decision reads text it can be steered by, so it is never a gate. + const push = taskOrThrow('push'); + expect(push.when).toEqual({ + anyOf: [ + "tasks.approve-push.output.decision == 'approved'", + "tasks.read-push-approval.output.branchPolicy.pushApproval == 'never'", + "tasks.review-commit.output.verdict == 'approve'", + ], + onMissingRef: 'skip', + }); + const reads = (task: WorkflowTask) => + JSON.stringify([task.when, task.inputBindings, task.dependsOn]); + for (const gate of ['push', 'approve-push', 'read-push-approval', 'scan-commit']) { + expect(reads(taskOrThrow(gate) as WorkflowTask), gate).not.toMatch(/decide-|fix-|republish-/); + } + }); +}); diff --git a/packages/platform-artifacts/src/skillCatalog/publishLocalChanges.ts b/packages/platform-artifacts/src/skillCatalog/publishLocalChanges.ts index dc5b43cb..62f58beb 100644 --- a/packages/platform-artifacts/src/skillCatalog/publishLocalChanges.ts +++ b/packages/platform-artifacts/src/skillCatalog/publishLocalChanges.ts @@ -1,4 +1,5 @@ import { + DECISION_MAX_EACH_ITEMS, HOST_PUSH_APPROVAL_DEFAULT, type HostPushApproval, MAX_PARENT_INPUTS_SERIALIZED_BYTES, @@ -9,6 +10,17 @@ import { type SkillCatalogEntry, toJsonSchemaSync, } from '@aflow/schemas'; +import { + AFTER_CHECK, + CHECK_FAILED, + CHECKS_FAILED_TASK, + DECIDE_AFTER_CHECK_TASK, + DECIDE_AFTER_REVIEW_TASK, + DECIDE_FINDINGS_TASK, + FIX_CHECKS_TASK, + FIX_FINDINGS_TASK, + REPUBLISH_FIX_TASK, +} from './publishLocalChangesRounds.js'; import { REVIEW_LOCAL_CHANGES } from './reviewLocalChanges.js'; /** @@ -61,6 +73,16 @@ const REVIEW_SKILL = { slug: REVIEW_LOCAL_CHANGES.bundle.workflow.slug, }; +/** + * Where the push may follow the checks: they passed on the first run, or on + * the second the decision after a failed first run asked for. Read by every + * task that only runs on a commit the checks cleared. + */ +const CHECKS_CLEARED = { + anyOf: ['tasks.check-commit.output.passed == true', 'tasks.recheck-commit.output.passed == true'], + onMissingRef: 'skip' as const, +}; + /** The one posture under which the publication reviews its commit to decide whether to ask. */ const REVIEW_GATED_POSTURE: HostPushApproval = 'unless-unreviewed'; @@ -141,9 +163,6 @@ const SUMMARY_SCHEMA = { type: 'string', minLength: 1, maxLength: 20_000 }; const FINDINGS_SCHEMA = { ...toJsonSchemaSync(PullRequestFindingsSchema) }; const GAPS_SCHEMA = { ...toJsonSchemaSync(PullRequestGapsSchema) }; -const CHECK_FAILED = - "The folder's checks failed on this run's commit: their account is above, with the end of what they printed, and the whole of it is stored with this failure. Nothing was scanned, reviewed or pushed, and the commit is still on its branch in the folder. Commission the fix onto this branch — nothing of it has left the machine — and publish again. Where they ran out of the folder's time rather than failed, the folder's `checksTimeoutMs` is the operator's to raise on the machine."; - /** Both runs of the checks take the same commit, measured against the same base. */ const CHECK_INPUT_BINDINGS = { bindingId: { kind: 'run_input' as const, path: 'bindingId' }, @@ -173,29 +192,29 @@ const CHECK_INPUT_TEMPLATE = { const PUBLISH_LOCAL_CHANGES: SkillCatalogEntry = { catalogId: 'publish-local-changes', - version: 24, + version: 25, name: 'Publish Local Changes', tagline: 'Commit a patch onto a branch of a connected repository, then push it and open the pull request — asking the operator first unless the folder says otherwise.', description: `Fits a request to publish an existing patch — a commission's result, or a diff the operator hands over — onto a branch of a connected repository and into a pull request. The working tree is never touched, and nothing leaves the machine until the push is cleared. -**Not for a folder whose machine block shows no publish prefix**: its push is refused. Say so, ask for the folder to be reconnected with one, and stop rather than commit what cannot be published. +**Not for a folder whose machine block shows no publish prefix**: its push is refused. Say so, ask for it to be reconnected with one, and stop. -**What it needs**: the connected folder; the change — for a commission's work, the \`patchRef\` its result reports, passed as \`patchRef\`, or for a diff the operator hands over, that text as \`patch\`, one or the other — or neither, for a branch as it stands; a branch name under the folder's publish prefix; the commit message; the repository owner and name, from the folder's \`origin\` remote; and the base branch the pull request targets. Ask for what is missing; never invent it. +**What it needs**: the connected folder; the change — a commission's \`patchRef\` as its result reports it, or a diff the operator hands over as \`patch\`, one or the other — or neither, for a branch as it stands; a branch name under the folder's publish prefix; the commit message; the repository owner and name, from the folder's \`origin\` remote; and the base branch the pull request targets. Ask for what is missing; never invent it. **The pull request is composed from the whole branch** and rewritten on each push; pass earlier reviews' \`findings\` and the commissions' \`gaps\`, and \`title\` or \`summary\` only for a person's own words. -**Never pass a commission's \`patch\` text.** It is a copy for reading, cut short on a large change, and the run's inputs are capped at ${String(RUN_INPUTS_KB)} KB together. \`patchRef\` names the whole diff at any size. +**Never pass a commission's \`patch\` text.** It is a copy for reading, cut short on a large change; \`patchRef\` names the whole diff at any size. -**When it asks before pushing**: the folder's push approval decides, and the machine block shows it as \`pushApproval\`. \`always\` asks before every push; \`never\` pushes without asking. \`unless-unreviewed\`: the run has Local Code Review read everything the push would add and pushes without asking only when it returns \`approve\` — on any other verdict, or a review that did not finish or could not start, it waits for the operator. The review is the catalog's as installed; an edited copy is refused, and the run asks. A folder that names no posture is \`${HOST_PUSH_APPROVAL_DEFAULT}\`, the default. Whatever the posture, the run scans every line the push would add, the headers and message of every commit it carries, and the pull request's title and summary for secrets before any of this, and a finding stops it with nothing pushed — which is what lets a review stand in for the operator. Only a range the scan cleared can go without asking: where a file was not read whole, or a line that looks like a secret ends in an \`aflow-scan: allow\` comment, no review runs and the run waits for the operator whatever the posture, \`never\` included, naming each and why. The comment never clears a line — whoever wrote the change could have written it; it turns a stop into a question. Once the run asks, the operator's answer decides: a declined push pushes nothing, whatever the posture. +**When it asks before pushing**: the folder's push approval, \`pushApproval\` in the machine block, decides. \`always\` asks before every push; \`never\` pushes without asking. \`unless-unreviewed\`: the run has Local Code Review read everything the push would add and pushes without asking only when it returns \`approve\` — otherwise, a review that did not finish or start included, it waits for the operator. The review is the catalog's as installed; an edited copy is refused, and the run asks. A folder that names no posture is \`${HOST_PUSH_APPROVAL_DEFAULT}\`, the default. Whatever the posture, the run scans every line the push would add, the headers and message of every commit it carries, and the pull request's title and summary for secrets before any of this, and a finding stops it with nothing pushed — which is what lets a review stand in for the operator. Only a range the scan cleared can go without asking: where a file was not read whole, or a line that looks like a secret ends in an \`aflow-scan: allow\` comment, no review runs and the run waits for the operator whatever the posture, \`never\` included. The comment never clears a line — whoever wrote the change could have written it; it turns a stop into a question. Once the run asks, the operator's answer decides: a declined push pushes nothing, whatever the posture. -**Its checks first**: the folder's \`checks\`, shown in the machine block, run in a checkout of the commit before the scan, and a failure stops the run with nothing scanned, reviewed or pushed; the push needs their passing receipt. A folder that declares none runs none. The pull request's own checks remain the proof. +**Its checks first**: the folder's \`checks\`, shown in the machine block, run in a checkout of the commit before the scan, and nothing is scanned, reviewed or pushed until they pass; the push needs their passing receipt. A folder that declares none runs none. The pull request's own checks remain the proof. A failure is decided on from what they printed — a rerun, a fix commissioned and published again, or a stop, as an unsure decision is. Review findings judged defects may take a fix round after the push. No decision clears or holds a push. **On an existing branch**: a fix commissioned from a branch (\`base: \`) is published onto it by naming it as \`branch\` and passing the commission's \`baseSha\` as \`baseSha\`; a branch is reused only that way, and a fresh change takes a fresh branch. A fix to a branch \`main\` has moved past is commissioned with \`mergeFrom: origin/\` as well and published with the sha the commission reported in \`merge.from\` as \`mergeFrom\`: the branch then carries one merge commit holding the fix. A commission that reported a \`merge\` and no \`patchRef\` changed nothing beyond the merge: it is published with \`mergeFrom\` and neither \`patchRef\` nor \`patch\`. **A branch as it stands**: a commit already made whose push did not land is published with \`branch\`, its head as \`baseSha\`, and no \`patchRef\`, \`patch\`, \`mergeFrom\` or \`commitMessage\`: nothing is committed, and the head is checked, scanned, reviewed and pushed. -**With the result**: report the pull request link, and whether the push was approved by the operator or cleared by the folder's push approval — and, where the run reviewed its commit, the verdict. A branch that already has an open pull request into the base gets no second one: the run reports that one. Where approval was declined, report that nothing was pushed and the commit stays on the machine to publish later. Where the checks failed, nothing left the machine: report the end of what they printed. Where the scan found what looks like a secret, the run failed with nothing pushed: report the files, commit headers and messages, title or summary, lines and rules it names — never ask for or repeat the value — and say the secret has to come out and the change be commissioned again onto a fresh branch, this one holding that commit. A refused push leaves the commit on its branch: report its message and the remedy the run names. Where the push failed, git's own message says why; a branch on \`origin\` that moved on is never overwritten.`, +**With the result**: report the pull request link, and whether the push was approved by the operator or cleared by the folder's push approval — and, where the run reviewed its commit, the verdict. A branch that already has an open pull request into the base gets no second one: the run reports that one. Where approval was declined, report that nothing was pushed and the commit stays on the machine to publish later. Where the checks failed, nothing left the machine: report the end of what they printed. A fix published again reports that run's pull request. Where the scan found what looks like a secret, the run failed with nothing pushed: report the files, commit headers and messages, title or summary, lines and rules it names — never ask for or repeat the value — and say the secret has to come out and the change go again on a fresh branch, this one holding that commit. A refused push leaves the commit on its branch: report its message and the remedy the run names. Where the push failed, git's own message says why.`, tags: ['coding', 'publish', 'git', 'local', 'developer-tools'], capabilityHints: [ { @@ -480,23 +499,21 @@ const PUBLISH_LOCAL_CHANGES: SkillCatalogEntry = { expression: "tasks.commit.output.state == 'applied'", onMissingRef: 'skip' as const, }, - // `clearedOrRetriableSha` exists only where the checks passed, none - // are declared, or tests running out of time alone failed them, so - // any other failure fails the projection; with one attempt and no - // output contract that fails the run here, the check's own summary - // — which carries the tail — leading the message. A run lost to - // time alone succeeds with `passed: false` and its `testTimeouts`, - // which is the record `recheck-commit` runs on. + // A run that did not pass succeeds with `passed: false` and its + // account, which is what `decide-after-check` reads to choose what + // follows; nothing after the checks runs unless they cleared. retryability: 'safe' as const, maxAttempts: 1, inputBindings: CHECK_INPUT_BINDINGS, context: CHECK_CONTEXT, inputTemplate: CHECK_INPUT_TEMPLATE, outputProjection: { - clearedOrRetriableSha: { path: 'clearedOrRetriableSha', onMissing: 'error' as const }, + clearedOrRetriableSha: { path: 'clearedOrRetriableSha', onMissing: 'null' as const }, passed: { path: 'passed', onMissing: 'error' as const }, skipped: { path: 'skipped', onMissing: 'null' as const }, + timedOut: { path: 'timedOut', onMissing: 'null' as const }, testTimeouts: { path: 'testTimeouts', onMissing: 'null' as const }, + failingGuard: { path: 'failingGuard', onMissing: 'null' as const }, summary: { path: 'summary', onMissing: 'error' as const }, tail: { path: 'tail', onMissing: 'null' as const }, // Null where the folder declares no checks: the push needs none then. @@ -504,18 +521,23 @@ const PUBLISH_LOCAL_CHANGES: SkillCatalogEntry = { }, }, + DECIDE_AFTER_CHECK_TASK, + + CHECKS_FAILED_TASK, + { taskId: 'recheck-commit', - name: "Run the folder's checks again, after tests ran out of time", - goal: "Run the folder's checks once more on this run's commit, where their first run failed only because tests ran out of their time — `testTimeouts` on the first run names them — which a machine under load does to a passing commit. This run's result is the one the push carries; a failure here fails the publication.", - failureInstruction: `${CHECK_FAILED} Their first run failed only because tests ran out of their time, and this second run failed as well.`, + name: "Run the folder's checks again, after they ran out of time", + goal: "Run the folder's checks once more on this run's commit, where the decision after their first run found nothing failed but time — which a machine under load does to a passing commit. This run's result is the one the push carries; a failure here fails the publication.", + failureInstruction: `${CHECK_FAILED} Their first run was judged to have failed only on time, and this second run failed as well.`, type: 'operation' as const, operation: 'host.commit.check', - dependsOn: ['commit', 'check-commit'], - // The first run succeeds without passing only where tests running - // out of time alone failed it. + dependsOn: ['commit', 'check-commit', 'decide-after-check'], when: { - expression: 'tasks.check-commit.output.passed == false', + allOf: [ + `${AFTER_CHECK}.nextStep == 'recheck_on_timeout'`, + `${AFTER_CHECK}.decided == true`, + ], onMissingRef: 'skip' as const, }, // Its own task rather than a second attempt of the first, which a @@ -545,10 +567,7 @@ const PUBLISH_LOCAL_CHANGES: SkillCatalogEntry = { // After the checks, and their second run where there is one: a // commit they failed goes no further. dependsOn: ['commit', 'check-commit', 'recheck-commit'], - when: { - expression: "tasks.commit.output.state == 'applied'", - onMissingRef: 'skip' as const, - }, + when: CHECKS_CLEARED, // `unflaggedRange` exists only when no rule matched, so a finding // fails the projection; with one attempt and no output contract that // fails the run here instead of parking it on a resolution that @@ -599,11 +618,8 @@ const PUBLISH_LOCAL_CHANGES: SkillCatalogEntry = { "The GitHub credential bound to this space cannot see `owner`/`repo` — GitHub answers 404 for a private repository the token has no access to — and that credential's repository access is the thing to check. Nothing has been pushed, and the commit is still on its branch.", type: 'operation' as const, operation: 'api.http.call', - dependsOn: ['commit'], - when: { - expression: "tasks.commit.output.state == 'applied'", - onMissingRef: 'skip' as const, - }, + dependsOn: ['commit', 'check-commit', 'recheck-commit'], + when: CHECKS_CLEARED, // One attempt and no output contract: a repository the credential // cannot see fails the run here, rather than parking it on a // resolution nobody can supply. @@ -726,13 +742,14 @@ const PUBLISH_LOCAL_CHANGES: SkillCatalogEntry = { outputProjection: { verdict: { path: 'result.output.verdict', onMissing: 'null' as const }, reviewSummary: { path: 'result.output.reviewSummary', onMissing: 'null' as const }, + findings: { path: 'result.output.findings', onMissing: 'null' as const }, outcome: { path: 'outcome', onMissing: 'error' as const }, reviewRunId: { path: 'runId', onMissing: 'error' as const }, }, outputContract: { schema: { type: 'object', - required: ['verdict', 'reviewSummary', 'outcome', 'reviewRunId'], + required: ['verdict', 'reviewSummary', 'findings', 'outcome', 'reviewRunId'], additionalProperties: false, properties: { verdict: { @@ -745,6 +762,14 @@ const PUBLISH_LOCAL_CHANGES: SkillCatalogEntry = { description: "The review's own account — every finding by severity and `file:line`, and the checks it ran — which the pull request carries; null when it did not complete.", }, + findings: { + type: ['array', 'null'], + items: { type: 'object' }, + // The review returns at most this many, and a decision asks of at most as many. + maxItems: DECISION_MAX_EACH_ITEMS, + description: + 'Every finding as the review returned it — severity, file, line, claim, evidence, suggestion — which the decision on a fix round reads one at a time; null when the review did not complete.', + }, outcome: { type: 'string', description: @@ -756,6 +781,10 @@ const PUBLISH_LOCAL_CHANGES: SkillCatalogEntry = { }, }, + DECIDE_FINDINGS_TASK, + + DECIDE_AFTER_REVIEW_TASK, + { taskId: 'describe-pr', name: 'Compose the pull request from the branch', @@ -765,10 +794,7 @@ const PUBLISH_LOCAL_CHANGES: SkillCatalogEntry = { type: 'operation' as const, operation: 'host.commit.describe', dependsOn: ['commit', 'check-commit', 'recheck-commit', 'scan-commit', 'review-commit'], - when: { - expression: "tasks.commit.output.state == 'applied'", - onMissingRef: 'skip' as const, - }, + when: CHECKS_CLEARED, retryability: 'safe' as const, maxAttempts: 1, // Whole outputs, each absent where its task did not run: a review @@ -1140,6 +1166,12 @@ const PUBLISH_LOCAL_CHANGES: SkillCatalogEntry = { { kind: 'output_path' as const, path: 'prNumber', toState: 'prNumber' }, ], }, + + FIX_CHECKS_TASK, + + FIX_FINDINGS_TASK, + + REPUBLISH_FIX_TASK, ], stateVariables: [ { @@ -1161,11 +1193,20 @@ const PUBLISH_LOCAL_CHANGES: SkillCatalogEntry = { immutable: false, writers: 'alternatives' as const, }, + { + variableId: 'afterReview', + name: 'After the review', + description: + "What the run decided about the review's findings judged to name defects: fix_before_merge, publish_as_is or ask — ask meaning the operator's word is wanted before the branch merges. Absent where no finding was judged a defect or no decision was made.", + required: false, + sensitive: false, + immutable: false, + }, ], output: { primary: 'prUrl', guidance: - 'The run carries prUrl and prNumber once the branch has its pull request — opened by this run, or already open for a branch it appended to — and reporting the link is reporting the result. Absent, the branch was committed and not published: say it stayed on the machine to publish later — unless the run failed at the checks, whose failure names what to fix, or at the scan, where the commit holds what looks like a secret and must not be published.', + "The run carries prUrl and prNumber once the branch has its pull request — opened, already open, or the one a fix it commissioned was published to — and reporting the link is the result; afterReview is its decision on the review's findings, ask wanting the operator's word. Absent prUrl, the branch stayed on the machine to publish later — unless the run failed at the checks, whose failure names what to fix, or at the scan, where the commit holds what looks like a secret.", }, iteration: { auto: false, maxConsecutiveRuns: 1, stopOnOutcomesMet: true, cooldownMs: 0 }, }, @@ -1206,7 +1247,7 @@ const PUBLISH_LOCAL_CHANGES: SkillCatalogEntry = { priority: 50, }, rationale: - "Thirteen tasks, the approval between local and published halves: the commit lands in a detached worktree. host.commit.check runs the folder's checks on it: a failure fails the run, bar test timeouts alone: those rerun once. host.commit.scan reads all a push would add (from a fetched origin/), commit headers, messages, PR text: a finding fails the run; an unread file or an allowed line asks, unreviewed. Over a cleared range the posture, and under unless-unreviewed a Local Code Review, decide whether to ask; asked, it obeys. The push argv is pinned bar the refspec: no force, tag or submodule. The executor re-measures origin/ as git runs, rescanning where it moved or a restart voids the receipt; declared checks need a passed receipt; an uncleared range goes only on this run's grant. host.commit.describe composes the PR from the whole range, reviews, checks and gaps, scanned, before the push; an open PR gets it by PATCH, not a twin.", + "Twenty tasks, the approval between local and published halves. The commit lands in a detached worktree; host.commit.check runs the folder's checks. A failed check goes to ai.decision.decide: a recheck, a fix commissioned from the checks' summary and republished, or workflow.run.fail with that summary, which is also what abstaining does. host.commit.scan reads all a push adds: a finding fails the run, an unread file or allowed line asks. Over a cleared range the posture, and under unless-unreviewed a Local Code Review, decide whether to ask. Its findings are judged one by one, and a choice over those passing commissions a fix and republishes after this push; abstaining fixes. No decision gates the push. The push argv is pinned bar the refspec; the executor re-measures origin/, needs a passed check receipt, and sends an uncleared range only on this run's grant. host.commit.describe composes the PR before the push; an open PR is PATCHed.", }, }; diff --git a/packages/platform-artifacts/src/skillCatalog/publishLocalChangesDecisions.ts b/packages/platform-artifacts/src/skillCatalog/publishLocalChangesDecisions.ts new file mode 100644 index 00000000..c80417b8 --- /dev/null +++ b/packages/platform-artifacts/src/skillCatalog/publishLocalChangesDecisions.ts @@ -0,0 +1,84 @@ +import type { DecisionChoiceQuestionSchema, DecisionYesNoQuestionSchema } from '@aflow/schemas'; +import type { z } from 'zod'; + +/** + * The two judgements Publish Local Changes makes for itself — what follows a + * failed check, and whether a review's findings are worth a fix round — as the + * questions it asks a decision model. Defined once, here, so the measurement + * of decision quality on the loop's own history (`yarn decisions:measure-loop`, + * Plan 316 §7) asks exactly what the skill asks. + * + * The thresholds are where each decision abstains and the run does what it did + * before it decided anything. Plan 316 §7 records the measurement they are to + * be read against; until it has run on the loop's records they are the plan's + * provisional values, set high so an unsure answer falls to the old path. + */ +export const AFTER_CHECK_MIN_CONFIDENCE = 0.8; +export const FINDING_MIN_CONFIDENCE = 0.8; +export const AFTER_REVIEW_MIN_CONFIDENCE = 0.9; + +export const AFTER_CHECK_ROUTES = [ + 'recheck_on_timeout', + 'fix_commission', + 'operator_install_mismatch', +] as const; +export type AfterCheckRoute = (typeof AFTER_CHECK_ROUTES)[number]; + +export const AFTER_REVIEW_ROUTES = ['fix_before_merge', 'publish_as_is', 'ask'] as const; +export type AfterReviewRoute = (typeof AFTER_REVIEW_ROUTES)[number]; + +/** The question name each decision's answer is read under: `answers.`. */ +export const AFTER_CHECK_QUESTION_NAME = 'next_step'; +export const AFTER_REVIEW_QUESTION_NAME = 'after_review'; + +export const AFTER_CHECK_QUESTION = { + type: 'choice', + instructions: + "The folder's checks did not pass on this publication's commit. From what they printed, what follows: running them again, a fix to the change, or the operator putting the machine right.", + options: { + recheck_on_timeout: + "Nothing failed but time: tests ran out of their time under a loaded machine, or the checks were still running when the folder's time ran out, and nothing printed is an assertion, a type error, a lint finding or a guard refusing the change. Run on the same commit again, they are expected to pass.", + fix_commission: + 'The checks found something wrong in the change itself — a failing assertion, a type error, a lint or format finding, a guard the change trips — which a coding agent can put right in the repository from what they printed.', + operator_install_mismatch: + 'The checks could not run as the folder declares them on this machine — a missing tool or dependency, a version out of step with the lockfile, a sandbox or permission refusal, a service the checks reach that is not there — which only the operator can put right on the machine.', + }, + minConfidence: AFTER_CHECK_MIN_CONFIDENCE, +} as const satisfies z.input; + +export const FINDING_QUESTION = { + type: 'yes_no', + instructions: + "The finding names a defect in the change's behaviour, established by evidence shown.", + criteria: { + true: 'The claim is about what the changed code does — a wrong result, a crash, a race, a leak, a hole, a broken contract or a test that does not test what it says — and the evidence quoted shows it in this change.', + false: + 'The claim is a preference, a name, a style or a wording; or a defect the evidence does not show; or about code this change did not touch.', + }, + minConfidence: FINDING_MIN_CONFIDENCE, +} as const satisfies z.input; + +export const AFTER_REVIEW_QUESTION = { + type: 'choice', + instructions: + 'These are the findings of the review of a branch about to be published that name defects in its behaviour. Whether they are fixed on this branch before it merges.', + options: { + fix_before_merge: + 'At least one of them should not merge as it is: a fix round on this branch is worth its cost before the branch merges.', + publish_as_is: + 'They are narrow enough to merge as they stand and be fixed later, if at all; the pull request records them.', + ask: 'Whether to fix them first turns on something only the operator knows — scope, a deadline, what the change was for.', + }, + minConfidence: AFTER_REVIEW_MIN_CONFIDENCE, +} as const satisfies z.input; + +/** + * The opening of each fix brief. What follows it is data the run already + * holds — the checks' own account, the findings that passed — so no model + * writes a brief and none can be argued with as one. + */ +export const FIX_CHECKS_BRIEF_LEAD = + "The folder's declared checks failed on this commit. Their account follows, naming the step that failed where they named one, with the end of what they printed. Make them pass by fixing what they found in the change; change no check, guard or test to pass it, and leave the change otherwise as it is. Run the failing step again in this checkout before you finish, and report what you changed and what it printed.\n\n"; + +export const FIX_FINDINGS_BRIEF_LEAD = + "Local Code Review read this branch, and these of its findings name defects in the change's behaviour, each with the evidence for it. Fix each of them in the change, with a test that would have caught it where one can; change nothing else. Run the folder's own checks over what you touched before you finish, and report, finding by finding, what you changed.\n\nThe findings, as the review returned them:\n"; diff --git a/packages/platform-artifacts/src/skillCatalog/publishLocalChangesRounds.ts b/packages/platform-artifacts/src/skillCatalog/publishLocalChangesRounds.ts new file mode 100644 index 00000000..cfc79da5 --- /dev/null +++ b/packages/platform-artifacts/src/skillCatalog/publishLocalChangesRounds.ts @@ -0,0 +1,445 @@ +/** + * What Publish Local Changes does with its two decisions (Plan 315 D22): the + * decision after a failed check and its stop, the judgement of a review's + * findings and the decision on a fix round, and the fix round itself — a + * commission and the publication of its fix. The questions are in + * `publishLocalChangesDecisions.ts`; where these tasks sit in the graph, and + * the push gate none of them touches, are in `publishLocalChanges.ts`. + */ +import { COMMISSION_CHANGE } from './commissionChange.js'; +import { + AFTER_CHECK_QUESTION, + AFTER_CHECK_QUESTION_NAME, + AFTER_REVIEW_QUESTION, + AFTER_REVIEW_QUESTION_NAME, + FINDING_QUESTION, + FIX_CHECKS_BRIEF_LEAD, + FIX_FINDINGS_BRIEF_LEAD, +} from './publishLocalChangesDecisions.js'; + +export const CHECK_FAILED = + "The folder's checks failed on this run's commit: their account is above, with the end of what they printed, and the whole of it is stored with this failure. Nothing was scanned, reviewed or pushed, and the commit is still on its branch in the folder. Commission the fix onto this branch — nothing of it has left the machine — and publish again. Where they ran out of the folder's time rather than failed, the folder's `checksTimeoutMs` is the operator's to raise on the machine."; + +/** The commission a fix round starts, and the publication that publishes its fix: each as the Store installed it. */ +const COMMISSION_SKILL = { + catalogId: COMMISSION_CHANGE.catalogId, + slug: COMMISSION_CHANGE.bundle.workflow.slug, +}; +const PUBLISH_SKILL = { + catalogId: 'publish-local-changes', + slug: 'publish-local-changes', +}; + +export const AFTER_CHECK = `tasks.decide-after-check.output`; +const AFTER_REVIEW = `tasks.decide-after-review.output`; + +/** A run started by a task, waited on, from the skill the Store installed. */ +const RUN_START_CONTEXT = { + strategy: 'scoped' as const, + contextPolicy: 'auto-optimize' as const, + learnings: 'none' as const, + capabilities: { + operations: ['workflow.run.start'], + integrations: [], + }, +}; + +const DECIDE_CONTEXT = { + strategy: 'scoped' as const, + contextPolicy: 'auto-optimize' as const, + learnings: 'none' as const, + capabilities: { + operations: ['ai.decision.decide'], + integrations: [], + }, +}; + +/** What a fix round's commission returns, read from its run: no change is no fix, and fails the round. */ +const FIX_COMMISSION_PROJECTION = { + patchRef: { path: 'result.output.patchRef', onMissing: 'error' as const }, + baseSha: { path: 'result.output.baseSha', onMissing: 'error' as const }, + outcome: { path: 'outcome', onMissing: 'error' as const }, + commissionRunId: { path: 'runId', onMissing: 'error' as const }, +}; + +/** What the publication of a fix ends with, promoted as this run's pull request. */ +const REPUBLISH_PROJECTION = { + prUrl: { path: 'result.output.prUrl', onMissing: 'null' as const }, + prNumber: { path: 'result.output.prNumber', onMissing: 'null' as const }, + outcome: { path: 'outcome', onMissing: 'error' as const }, + republishRunId: { path: 'runId', onMissing: 'error' as const }, +}; + +const REPUBLISH_PROMOTES = [ + { kind: 'output_path' as const, path: 'prUrl', toState: 'prUrl' }, + { kind: 'output_path' as const, path: 'prNumber', toState: 'prNumber' }, +]; + +/** What a publication of a fix carries on from this one besides the fix itself. */ +const REPUBLISH_PASSED_ON_BINDINGS = { + bindingId: { kind: 'run_input' as const, path: 'bindingId' }, + branch: { kind: 'run_input' as const, path: 'branch' }, + owner: { kind: 'run_input' as const, path: 'owner' }, + repo: { kind: 'run_input' as const, path: 'repo' }, + base: { kind: 'run_input' as const, path: 'base' }, + findings: { kind: 'run_input' as const, path: 'findings' }, + gaps: { kind: 'run_input' as const, path: 'gaps' }, + commitSha: { kind: 'task_output' as const, taskId: 'commit', path: 'commit.sha' }, +}; + +const REPUBLISH_PASSED_ON_INPUTS = { + bindingId: { $bind: 'bindingId' }, + branch: { $bind: 'branch' }, + owner: { $bind: 'owner' }, + repo: { $bind: 'repo' }, + base: { $bind: 'base' }, + findings: { $bind: 'findings' }, + gaps: { $bind: 'gaps' }, +}; + +export const DECIDE_AFTER_CHECK_TASK = { + taskId: 'decide-after-check', + name: 'Decide what follows the failed checks', + goal: "Read the account of the folder's checks that did not pass — their summary, the failing step where they named one, the end of what they printed — and choose what follows: run them again where nothing failed but time, commission a fix where they found something wrong in the change, or stop for the operator where the machine cannot run them as declared.", + type: 'operation' as const, + operation: 'ai.decision.decide', + dependsOn: ['check-commit'], + when: { + expression: 'tasks.check-commit.output.passed == false', + onMissingRef: 'skip' as const, + }, + // A decision that cannot be made — no decision model bound to the + // space, a provider refusing — is no answer: `checks-failed` reads + // its status and stops the run as a failed check always did. + optional: true, + retryability: 'safe' as const, + maxAttempts: 2, + inputBindings: { + summary: { kind: 'task_output' as const, taskId: 'check-commit', path: 'summary' }, + tail: { kind: 'task_output' as const, taskId: 'check-commit', path: 'tail' }, + failingGuard: { + kind: 'task_output' as const, + taskId: 'check-commit', + path: 'failingGuard', + }, + timedOut: { kind: 'task_output' as const, taskId: 'check-commit', path: 'timedOut' }, + testTimeouts: { + kind: 'task_output' as const, + taskId: 'check-commit', + path: 'testTimeouts', + }, + }, + context: DECIDE_CONTEXT, + inputTemplate: { + state: { + summary: { $bind: 'summary' }, + tail: { $bind: 'tail' }, + failingGuard: { $bind: 'failingGuard' }, + timedOut: { $bind: 'timedOut' }, + testTimeouts: { $bind: 'testTimeouts' }, + }, + questions: { [AFTER_CHECK_QUESTION_NAME]: AFTER_CHECK_QUESTION }, + }, + outputProjection: { + nextStep: { + path: `answers.${AFTER_CHECK_QUESTION_NAME}.value`, + onMissing: 'error' as const, + }, + decided: { + path: `answers.${AFTER_CHECK_QUESTION_NAME}.decided`, + onMissing: 'error' as const, + }, + confidence: { + path: `answers.${AFTER_CHECK_QUESTION_NAME}.confidence`, + onMissing: 'error' as const, + }, + }, +}; + +export const CHECKS_FAILED_TASK = { + taskId: 'checks-failed', + name: 'Stop on the failed checks', + goal: "Fail the publication with the checks' own account, where the decision after them found the machine cannot run them as declared, could not decide, or could not be made — so whoever started it is woken with what the checks printed, as a failed check always woke them.", + failureInstruction: CHECK_FAILED, + type: 'operation' as const, + operation: 'workflow.run.fail', + dependsOn: ['check-commit', 'decide-after-check'], + when: { + anyOf: [ + `${AFTER_CHECK}.nextStep == 'operator_install_mismatch'`, + `${AFTER_CHECK}.decided == false`, + "tasks.decide-after-check.status == 'failed'", + ], + onMissingRef: 'skip' as const, + }, + retryability: 'safe' as const, + maxAttempts: 1, + inputBindings: { + summary: { kind: 'task_output' as const, taskId: 'check-commit', path: 'summary' }, + }, + context: { + strategy: 'scoped' as const, + contextPolicy: 'auto-optimize' as const, + learnings: 'none' as const, + capabilities: { + operations: ['workflow.run.fail'], + integrations: [], + }, + }, + inputTemplate: { + reason: { $bind: 'summary' }, + }, +}; + +export const DECIDE_FINDINGS_TASK = { + taskId: 'decide-findings', + name: "Judge each of the review's findings", + goal: "Ask of every finding this run's review returned, one at a time, whether it names a defect in the change's behaviour established by the evidence it shows — so a fix round is briefed with those alone, and never with a preference or a claim its evidence does not carry.", + type: 'operation' as const, + operation: 'ai.decision.decide', + dependsOn: ['review-commit'], + // A review with no findings, or one that did not finish, has none to judge. + when: { + expression: 'tasks.review-commit.output.findings[0] != null', + onMissingRef: 'skip' as const, + }, + // No decision is no fix round: the run goes on as a review's + // findings always left it, to whoever reads them. + optional: true, + retryability: 'safe' as const, + maxAttempts: 2, + inputBindings: { + verdict: { kind: 'task_output' as const, taskId: 'review-commit', path: 'verdict' }, + reviewSummary: { + kind: 'task_output' as const, + taskId: 'review-commit', + path: 'reviewSummary', + }, + findings: { kind: 'task_output' as const, taskId: 'review-commit', path: 'findings' }, + }, + context: DECIDE_CONTEXT, + inputTemplate: { + state: { + verdict: { $bind: 'verdict' }, + reviewSummary: { $bind: 'reviewSummary' }, + }, + each: { + items: { $bind: 'findings' }, + question: FINDING_QUESTION, + }, + }, + outputProjection: { + selected: { path: 'each.selected', onMissing: 'error' as const }, + selectedCount: { path: 'each.selectedCount', onMissing: 'error' as const }, + undecidedCount: { path: 'each.undecidedCount', onMissing: 'error' as const }, + selectedJson: { path: 'each.selectedJson', onMissing: 'error' as const }, + }, +}; + +export const DECIDE_AFTER_REVIEW_TASK = { + taskId: 'decide-after-review', + name: 'Decide whether the findings are fixed before merging', + goal: "Read the findings judged to name defects and choose whether this branch takes a fix round before it merges, merges as it is with the findings recorded, or waits on the operator's word.", + type: 'operation' as const, + operation: 'ai.decision.decide', + dependsOn: ['decide-findings'], + when: { + expression: 'tasks.decide-findings.output.selectedCount > 0', + onMissingRef: 'skip' as const, + }, + optional: true, + retryability: 'safe' as const, + maxAttempts: 2, + inputBindings: { + verdict: { kind: 'task_output' as const, taskId: 'review-commit', path: 'verdict' }, + findings: { kind: 'task_output' as const, taskId: 'decide-findings', path: 'selected' }, + }, + context: DECIDE_CONTEXT, + inputTemplate: { + state: { + verdict: { $bind: 'verdict' }, + findings: { $bind: 'findings' }, + }, + questions: { [AFTER_REVIEW_QUESTION_NAME]: AFTER_REVIEW_QUESTION }, + }, + outputProjection: { + afterReview: { + path: `answers.${AFTER_REVIEW_QUESTION_NAME}.value`, + onMissing: 'error' as const, + }, + decided: { + path: `answers.${AFTER_REVIEW_QUESTION_NAME}.decided`, + onMissing: 'error' as const, + }, + confidence: { + path: `answers.${AFTER_REVIEW_QUESTION_NAME}.confidence`, + onMissing: 'error' as const, + }, + }, + promoteOutputs: [{ kind: 'output_path' as const, path: 'afterReview', toState: 'afterReview' }], +}; + +export const FIX_CHECKS_TASK = { + taskId: 'fix-checks', + name: 'Commission the fix the checks call for', + goal: "Start a commission on this run's commit, briefed with the checks' own account — the step that failed and the end of what they printed — and wait for its change.", + failureInstruction: + "The commission briefed to make the folder's checks pass returned no change, or did not finish; its run says why. Nothing was scanned, reviewed or pushed, and the commit is still on its branch in the folder. Read the commission's run, then commission the fix onto this branch and publish again — or, where its brief was refused for its length, brief it by hand from the checks' account above.", + type: 'operation' as const, + operation: 'workflow.run.start', + dependsOn: ['commit', 'check-commit', 'decide-after-check'], + when: { + allOf: [`${AFTER_CHECK}.nextStep == 'fix_commission'`, `${AFTER_CHECK}.decided == true`], + onMissingRef: 'skip' as const, + }, + // A second attempt starts a second commission. + retryability: 'unsafe' as const, + maxAttempts: 1, + inputBindings: { + bindingId: { kind: 'run_input' as const, path: 'bindingId' }, + commitSha: { kind: 'task_output' as const, taskId: 'commit', path: 'commit.sha' }, + summary: { kind: 'task_output' as const, taskId: 'check-commit', path: 'summary' }, + }, + context: RUN_START_CONTEXT, + inputTemplate: { + slug: COMMISSION_SKILL.slug, + catalogId: COMMISSION_SKILL.catalogId, + inputs: { + bindingId: { $bind: 'bindingId' }, + task: { $concat: [FIX_CHECKS_BRIEF_LEAD, { $bind: 'summary' }] }, + base: { $bind: 'commitSha' }, + }, + wait: 'until_complete', + }, + outputProjection: FIX_COMMISSION_PROJECTION, +}; + +export const FIX_FINDINGS_TASK = { + taskId: 'fix-findings', + name: 'Commission the fix the findings call for', + goal: "Start a commission on this run's commit, briefed with the review's findings judged to name defects, as the review returned them, and wait for its change.", + failureInstruction: + "The commission briefed to fix the review's findings returned no change, or did not finish; its run says why. This run's commit went on as the folder's push approval says, and the findings are on the review. Read the commission's run, then commission the fix onto this branch and publish again — or, where its brief was refused for its length, brief it by hand from the findings.", + type: 'operation' as const, + operation: 'workflow.run.start', + dependsOn: ['commit', 'decide-findings', 'decide-after-review'], + // An undecided answer is the fix round: below the bar, fixing what + // was judged a defect costs a commission, and merging it costs more. + when: { + anyOf: [ + `${AFTER_REVIEW}.afterReview == 'fix_before_merge'`, + `${AFTER_REVIEW}.decided == false`, + ], + onMissingRef: 'skip' as const, + }, + retryability: 'unsafe' as const, + maxAttempts: 1, + inputBindings: { + bindingId: { kind: 'run_input' as const, path: 'bindingId' }, + commitSha: { kind: 'task_output' as const, taskId: 'commit', path: 'commit.sha' }, + selectedJson: { + kind: 'task_output' as const, + taskId: 'decide-findings', + path: 'selectedJson', + }, + }, + context: RUN_START_CONTEXT, + inputTemplate: { + slug: COMMISSION_SKILL.slug, + catalogId: COMMISSION_SKILL.catalogId, + inputs: { + bindingId: { $bind: 'bindingId' }, + task: { $concat: [FIX_FINDINGS_BRIEF_LEAD, { $bind: 'selectedJson' }] }, + base: { $bind: 'commitSha' }, + }, + wait: 'until_complete', + }, + outputProjection: FIX_COMMISSION_PROJECTION, +}; + +export const REPUBLISH_FIX_TASK = { + taskId: 'republish-fix', + name: 'Publish the fix', + goal: "Publish the fix a commission of this run made onto this branch, as a publication of its own — committed, checked, scanned, reviewed and pushed under the folder's push approval as any publication is — once this run's push has gone or was never cleared to, never beside it; and report its pull request as this run's.", + type: 'operation' as const, + operation: 'workflow.run.start', + // At most one fix round runs: the checks' only where they failed, + // the findings' only where they passed. After the push, since the fix + // is appended to this commit and two pushes of one branch must not + // race; a declined approval skips the push and everything after it, + // this included — the operator said no. + dependsOn: ['fix-checks', 'fix-findings', 'push'], + when: { + anyOf: [ + 'tasks.fix-checks.output.patchRef != null', + 'tasks.fix-findings.output.patchRef != null', + ], + onMissingRef: 'skip' as const, + }, + retryability: 'unsafe' as const, + maxAttempts: 1, + inputBindings: { + ...REPUBLISH_PASSED_ON_BINDINGS, + checksPatchRef: { + kind: 'task_output' as const, + taskId: 'fix-checks', + path: 'patchRef', + }, + checksBaseSha: { kind: 'task_output' as const, taskId: 'fix-checks', path: 'baseSha' }, + checksRunId: { + kind: 'task_output' as const, + taskId: 'fix-checks', + path: 'commissionRunId', + }, + findingsPatchRef: { + kind: 'task_output' as const, + taskId: 'fix-findings', + path: 'patchRef', + }, + findingsBaseSha: { + kind: 'task_output' as const, + taskId: 'fix-findings', + path: 'baseSha', + }, + findingsRunId: { + kind: 'task_output' as const, + taskId: 'fix-findings', + path: 'commissionRunId', + }, + }, + context: RUN_START_CONTEXT, + inputTemplate: { + slug: PUBLISH_SKILL.slug, + catalogId: PUBLISH_SKILL.catalogId, + inputs: { + ...REPUBLISH_PASSED_ON_INPUTS, + patchRef: { $firstOf: [{ $bind: 'checksPatchRef' }, { $bind: 'findingsPatchRef' }] }, + baseSha: { $firstOf: [{ $bind: 'checksBaseSha' }, { $bind: 'findingsBaseSha' }] }, + commitMessage: { + $firstOf: [ + { + $concat: [ + "Make the folder's checks pass\n\nThe fix commission ", + { $bind: 'checksRunId' }, + " made from what the folder's checks printed when they failed on ", + { $bind: 'commitSha' }, + '.', + ], + }, + { + $concat: [ + "Fix the review's findings\n\nThe fix commission ", + { $bind: 'findingsRunId' }, + ' made for the findings of the review of ', + { $bind: 'commitSha' }, + ' judged to name defects in its behaviour.', + ], + }, + ], + }, + }, + wait: 'until_complete', + }, + outputProjection: REPUBLISH_PROJECTION, + promoteOutputs: REPUBLISH_PROMOTES, +}; diff --git a/packages/platform-artifacts/src/skillCatalog/reviewLocalChanges.test.ts b/packages/platform-artifacts/src/skillCatalog/reviewLocalChanges.test.ts index 30899091..14f99953 100644 --- a/packages/platform-artifacts/src/skillCatalog/reviewLocalChanges.test.ts +++ b/packages/platform-artifacts/src/skillCatalog/reviewLocalChanges.test.ts @@ -106,12 +106,14 @@ describe('Review Local Changes — the first harness inside a skill', () => { expect(REVIEW_LOCAL_CHANGES.description).toContain('not something to ask the coding agent for'); }); - it('promotes the verdict and the summary only — a promoted array reads as a preview', () => { + it('promotes the verdict, the summary a person reads, and the findings a publication decides on one at a time', () => { const promoted = (task?.promoteOutputs ?? []).map((p) => ('toState' in p ? p.toState : '')); - expect(promoted).toEqual(['verdict', 'reviewSummary']); + expect(promoted).toEqual(['verdict', 'reviewSummary', 'findings']); const declared = (wf.stateVariables ?? []).map((v) => v.variableId); - expect(declared).toEqual(['verdict', 'reviewSummary']); + expect(declared).toEqual(['verdict', 'reviewSummary', 'findings']); for (const id of promoted) expect(declared).toContain(id); + // The summary stays the review as it is reported; the findings are for a run to act on. + expect(wf.output?.primary).toBe('verdict'); }); it('makes the summary self-sufficient, in the schema and in the task prose', () => { diff --git a/packages/platform-artifacts/src/skillCatalog/reviewLocalChanges.ts b/packages/platform-artifacts/src/skillCatalog/reviewLocalChanges.ts index 53bc6689..f673433c 100644 --- a/packages/platform-artifacts/src/skillCatalog/reviewLocalChanges.ts +++ b/packages/platform-artifacts/src/skillCatalog/reviewLocalChanges.ts @@ -143,7 +143,7 @@ const BRIEF_MAX_TURNS = 8; const REVIEW_LOCAL_CHANGES: SkillCatalogEntry = { catalogId: 'review-local-changes', - version: 7, + version: 8, name: 'Review Local Changes', tagline: "Review committed changes in a connected repository with the machine's own coding agent.", @@ -257,6 +257,7 @@ const REVIEW_LOCAL_CHANGES: SkillCatalogEntry = { promoteOutputs: [ { kind: 'output_path' as const, path: 'result.verdict', toState: 'verdict' }, { kind: 'output_path' as const, path: 'result.summary', toState: 'reviewSummary' }, + { kind: 'output_path' as const, path: 'result.findings', toState: 'findings' }, ], }, ], @@ -278,6 +279,15 @@ const REVIEW_LOCAL_CHANGES: SkillCatalogEntry = { sensitive: false, immutable: false, }, + { + variableId: 'findings', + name: 'Review findings', + description: + 'Every finding as the review returned it — severity, file, line, claim, evidence and suggestion — for a run that acts on them one at a time, as a publication deciding on a fix round does.', + required: false, + sensitive: false, + immutable: false, + }, ], output: { primary: 'verdict', diff --git a/packages/platform-artifacts/src/storeCatalogHashes.json b/packages/platform-artifacts/src/storeCatalogHashes.json index 2de6b025..1b7e2749 100644 --- a/packages/platform-artifacts/src/storeCatalogHashes.json +++ b/packages/platform-artifacts/src/storeCatalogHashes.json @@ -96,7 +96,7 @@ "contentHash": "5f9151872e93be3bb7ad3b154b9a539677892df0361ad01c2579f227a27b6856" }, "local-code-review": { - "version": 7, + "version": 8, "contentHash": "cd6f27ade9b8f319bd43cafed7e48afffb8412148560638f5181fabb505f8815" }, "local-commission": { @@ -104,7 +104,7 @@ "contentHash": "35998b7e73fb024809520f7738c4f169e18858a800d924baf195f43e6648a90a" }, "local-publish": { - "version": 21, + "version": 22, "contentHash": "4c46d0e167830967d1be26f00f20b0bdb21d36bac7375d1933ba5ff2cc74ac24" }, "newsapi": { diff --git a/packages/schemas/src/operations/aiDecision.test.ts b/packages/schemas/src/operations/aiDecision.test.ts index 19320f13..74628756 100644 --- a/packages/schemas/src/operations/aiDecision.test.ts +++ b/packages/schemas/src/operations/aiDecision.test.ts @@ -49,6 +49,35 @@ describe('AiDecideInputSchema', () => { expect(parse({}).success).toBe(false); }); + it('takes a question of each item alone, named questions alone, or both — never neither', () => { + const each = { items: [{ claim: 'a' }], question: { type: 'yes_no', instructions: 'Defect' } }; + const named = { u: { type: 'yes_no', instructions: 'x' } }; + expect(AiDecideInputSchema.safeParse({ state: 'x', each }).success).toBe(true); + expect(AiDecideInputSchema.safeParse({ state: 'x', each, questions: named }).success).toBe( + true, + ); + expect(AiDecideInputSchema.safeParse({ state: 'x' }).success).toBe(false); + }); + + it('refuses a named question under the name an item is asked by, and an each that cannot be asked', () => { + const each = { items: [{ claim: 'a' }], question: { type: 'yes_no', instructions: 'Defect' } }; + const clash = { each_0: { type: 'yes_no', instructions: 'x' } }; + expect(AiDecideInputSchema.safeParse({ state: 'x', each, questions: clash }).success).toBe( + false, + ); + for (const bad of [ + { ...each, question: { type: 'yes_no' } }, + { ...each, items: Array(51).fill({}) }, + ]) { + expect(AiDecideInputSchema.safeParse({ state: 'x', each: bad }).success).toBe(false); + } + }); + + it('takes an empty list, which a step reads only when the list it binds came back empty', () => { + const each = { items: [], question: { type: 'yes_no', instructions: 'Defect' } }; + expect(AiDecideInputSchema.safeParse({ state: 'x', each }).success).toBe(true); + }); + it('refuses a minConfidence outside 0..1', () => { expect(parse({ u: { type: 'yes_no', instructions: 'x', minConfidence: 1.5 } }).success).toBe( false, diff --git a/packages/schemas/src/operations/aiDecision.ts b/packages/schemas/src/operations/aiDecision.ts index 52453c40..c1850e5a 100644 --- a/packages/schemas/src/operations/aiDecision.ts +++ b/packages/schemas/src/operations/aiDecision.ts @@ -120,15 +120,77 @@ export const DecisionQuestionsSchema = z ); export type DecisionQuestions = z.infer; -export const AiDecideInputSchema = z.object({ - state: DecisionEntrySchema.describe( - 'What the questions are about: text, a JSON object or a JSON array. Content in the state can steer the answers, so a decision about untrusted input is never the only gate on something that matters.', - ), - questions: DecisionQuestionsSchema, - model: enumWithCustom(DECISION_MODELS) - .describe('Decision model to use. The platform default when absent.') - .optional(), -}); +/** The most items one `each` asks about: a review returns at most this many findings. */ +export const DECISION_MAX_EACH_ITEMS = 50; + +/** + * The name an `each` item's question travels under beside the named questions. + * A named question may not take it, so the two can never answer for each other. + */ +export const DECISION_EACH_QUESTION_PREFIX = 'each_'; +const EACH_QUESTION_NAME = /^each_\d+$/; + +export function decisionEachQuestionName(index: number): string { + return `${DECISION_EACH_QUESTION_PREFIX}${String(index)}`; +} + +export const DecisionEachSchema = z + .object({ + items: z + .array(z.unknown()) + .max(DECISION_MAX_EACH_ITEMS) + .describe( + 'The list the question is asked of, one item at a time — the findings of a review, the lines of an order. Each item is read beside the state. An empty list selects nothing, and with no named questions beside it nothing is asked.', + ), + question: DecisionYesNoQuestionSchema.describe( + 'The yes_no asked of every item, stated about one item. Its minConfidence holds for each answer.', + ), + }) + .superRefine((each, ctx) => { + const q = each.question; + if (q.instructions !== undefined || q.criteria?.true != null || q.criteria?.false != null) { + return; + } + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['question'], + message: + 'A yes_no question states the claim it asks about: give instructions, or criteria for true or false.', + }); + }); +export type DecisionEach = z.infer; + +export const AiDecideInputSchema = z + .object({ + state: DecisionEntrySchema.describe( + 'What the questions are about: text, a JSON object or a JSON array. Content in the state can steer the answers, so a decision about untrusted input is never the only gate on something that matters.', + ), + questions: DecisionQuestionsSchema.optional(), + each: DecisionEachSchema.optional().describe( + 'One yes_no asked of every item of a list, for a list whose length is known only when the step runs. The items it holds true for, confidently, come back as `each.selected` — and as `each.selectedJson`, text a later step can hand on.', + ), + model: enumWithCustom(DECISION_MODELS) + .describe('Decision model to use. The platform default when absent.') + .optional(), + }) + .superRefine((input, ctx) => { + if (input.questions === undefined && input.each === undefined) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['questions'], + message: 'Ask named questions, a question of each item, or both.', + }); + } + if (input.each === undefined) return; + for (const name of Object.keys(input.questions ?? {})) { + if (!EACH_QUESTION_NAME.test(name)) continue; + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['questions', name], + message: `"${name}" is how an item of \`each\` is asked; name the question otherwise.`, + }); + } + }); export type AiDecideInput = z.infer; const decidedSchema = z @@ -167,6 +229,8 @@ export const DecisionYesNoAnswerSchema = z.object({ decided: decidedSchema, }); +export type DecisionYesNoAnswer = z.infer; + export const DecisionAnswerSchema = z.discriminatedUnion('type', [ DecisionChoiceAnswerSchema, DecisionScoreAnswerSchema, @@ -174,8 +238,32 @@ export const DecisionAnswerSchema = z.discriminatedUnion('type', [ ]); export type DecisionAnswer = z.infer; +export const DecisionEachOutputSchema = z.object({ + answers: z + .array(DecisionYesNoAnswerSchema) + .describe('The answer for each item, in the order the items were given.'), + selected: z + .array(z.unknown()) + .describe('The items whose answer is true and decided, in their order.'), + selectedCount: z.number().int().nonnegative(), + undecidedCount: z + .number() + .int() + .nonnegative() + .describe('How many items were answered below the question’s minConfidence.'), + selectedJson: z + .string() + .describe( + '`selected` as indented JSON, for a step that takes text — a brief, a message. `[]` where none was selected.', + ), +}); +export type DecisionEachOutput = z.infer; + export const AiDecideOutputSchema = z.object({ - answers: z.record(z.string(), DecisionAnswerSchema).describe('Question name → answer.'), + answers: z + .record(z.string(), DecisionAnswerSchema) + .describe('Question name → answer. Empty where only `each` was asked.'), + each: DecisionEachOutputSchema.optional().describe('Present where `each` was asked.'), model: z.string(), usage: TokenUsageSchema, latencyMs: z.number().int().nonnegative(), diff --git a/packages/schemas/src/operations/hostCommitCheck.ts b/packages/schemas/src/operations/hostCommitCheck.ts index 08fce053..a2bcfce5 100644 --- a/packages/schemas/src/operations/hostCommitCheck.ts +++ b/packages/schemas/src/operations/hostCommitCheck.ts @@ -40,6 +40,9 @@ export const HOST_CHECK_OUTPUT_TAIL_BYTES = 1024 * 1024; */ export const HOST_CHECK_TEST_TIMEOUTS_MAX = 20; +/** The longest step name a check's report may give as the one that failed. */ +export const HOST_CHECK_FAILING_GUARD_MAX_CHARS = 1000; + export const HostCommitCheckInputSchema = z.object({ bindingId: HostBindingRef, sha: HostCommitShaSchema.describe( @@ -121,6 +124,16 @@ export const HostCommitCheckOutputSchema = z.object({ 'names, that nothing else failed — what a machine under load does to a passing commit. ' + 'At most `HOST_CHECK_TEST_TIMEOUTS_MAX`; the summary counts the rest.', ), + failingGuard: z + .string() + .min(1) + .max(HOST_CHECK_FAILING_GUARD_MAX_CHARS) + .optional() + .describe( + 'The step the checks name as the one that failed — a guard, a type check, a test run — ' + + 'as they reported it in the file `AFLOW_CHECK_REPORT` names. Present only where the ' + + 'checks did not pass and said which step it was; a fix is briefed with it.', + ), clearedOrRetriableSha: z .string() .optional() diff --git a/packages/schemas/src/operations/workflow/index.ts b/packages/schemas/src/operations/workflow/index.ts index fd594ca7..e05dd9f6 100644 --- a/packages/schemas/src/operations/workflow/index.ts +++ b/packages/schemas/src/operations/workflow/index.ts @@ -15,6 +15,7 @@ export * from './manage.js'; export * from './runStart.js'; export * from './runResume.js'; export * from './runCancelOp.js'; +export * from './runFailOp.js'; export * from './suggestedAction.js'; export * from './runDetail.js'; export * from './runAttention.js'; @@ -57,6 +58,7 @@ import { import { WorkflowRunStartInputSchema, WorkflowRunStartOutputSchema } from './runStart.js'; import { WorkflowRunResumeInputSchema, WorkflowRunResumeOutputSchema } from './runResume.js'; import { WorkflowRunCancelInputSchema, WorkflowRunCancelOutputSchema } from './runCancelOp.js'; +import { WorkflowRunFailInputSchema, WorkflowRunFailOutputSchema } from './runFailOp.js'; import { WorkflowRunDetailInputSchema, WorkflowRunDetailOutputSchema } from './runDetail.js'; import { WorkflowRunListAttentionInputSchema, @@ -95,6 +97,8 @@ export const WorkflowOperationRegistrations = createWorkflowOperationRegistratio WorkflowRunResumeOutputSchema, WorkflowRunCancelInputSchema, WorkflowRunCancelOutputSchema, + WorkflowRunFailInputSchema, + WorkflowRunFailOutputSchema, WorkflowRunDetailInputSchema, WorkflowRunDetailOutputSchema, WorkflowRunListAttentionInputSchema, diff --git a/packages/schemas/src/operations/workflow/registrations.ts b/packages/schemas/src/operations/workflow/registrations.ts index f45f1e01..582d1ab8 100644 --- a/packages/schemas/src/operations/workflow/registrations.ts +++ b/packages/schemas/src/operations/workflow/registrations.ts @@ -21,6 +21,10 @@ import type { WorkflowRunCancelInputSchema as WorkflowRunCancelInputSchemaT, WorkflowRunCancelOutputSchema as WorkflowRunCancelOutputSchemaT, } from './runCancelOp.js'; +import type { + WorkflowRunFailInputSchema as WorkflowRunFailInputSchemaT, + WorkflowRunFailOutputSchema as WorkflowRunFailOutputSchemaT, +} from './runFailOp.js'; import type { WorkflowRunDetailInputSchema as WorkflowRunDetailInputSchemaT, WorkflowRunDetailOutputSchema as WorkflowRunDetailOutputSchemaT, @@ -71,6 +75,8 @@ interface WorkflowRegistrationSchemas { WorkflowRunResumeOutputSchema: typeof WorkflowRunResumeOutputSchemaT; WorkflowRunCancelInputSchema: typeof WorkflowRunCancelInputSchemaT; WorkflowRunCancelOutputSchema: typeof WorkflowRunCancelOutputSchemaT; + WorkflowRunFailInputSchema: typeof WorkflowRunFailInputSchemaT; + WorkflowRunFailOutputSchema: typeof WorkflowRunFailOutputSchemaT; WorkflowRunDetailInputSchema: typeof WorkflowRunDetailInputSchemaT; WorkflowRunDetailOutputSchema: typeof WorkflowRunDetailOutputSchemaT; WorkflowRunListAttentionInputSchema: typeof WorkflowRunListAttentionInputSchemaT; @@ -113,6 +119,8 @@ export function createWorkflowOperationRegistrations( WorkflowRunResumeOutputSchema, WorkflowRunCancelInputSchema, WorkflowRunCancelOutputSchema, + WorkflowRunFailInputSchema, + WorkflowRunFailOutputSchema, WorkflowRunDetailInputSchema, WorkflowRunDetailOutputSchema, WorkflowRunListAttentionInputSchema, @@ -506,6 +514,42 @@ export function createWorkflowOperationRegistrations( inputZod: WorkflowRunCancelInputSchema, outputZod: WorkflowRunCancelOutputSchema, }, + { + stepType: 'workflow', + group: 'run', + verb: 'fail', + name: 'Fail Workflow Run', + agentTool: false, + actionLabel: 'Stopping the run\u2026', + semanticDescription: + 'Fail the run this task belongs to, with a stated reason, where the skill decided that ' + + 'what an earlier task returned ends it — the branch a decision takes when it cannot go ' + + 'on, written once in the skill. The step always fails, non-retryable, with the reason as ' + + 'its message and the task\u2019s failureInstruction beside it, so the run ends as one whose ' + + 'task failed and wakes whoever started it.', + tags: ['workflow', 'run', 'fail'], + idempotency: 'idempotent', + mutates: false, + usage: { + oneLine: 'End the run as failed, with a reason bound from what decided it.', + whenToUse: [ + 'A decision task routed to a branch that cannot be recovered within the skill', + 'A decision abstained, and the case is one a person or an agent outside the run must take', + ], + whenNotToUse: [ + 'An operation that fails on its own already fails the run — bind nothing to restate it', + 'Stopping another run — use workflow.run.cancel', + ], + pitfalls: [ + 'Gate it with a when: unconditioned, it fails every run that reaches it', + 'The reason is read by whoever is woken; bind it from the output that decided, not a fixed sentence', + ], + minimalExampleInput: { reason: 'The checks failed: tsc apps/x/tsconfig.json.' }, + }, + accessMode: 'read', + inputZod: WorkflowRunFailInputSchema, + outputZod: WorkflowRunFailOutputSchema, + }, { stepType: 'workflow', group: 'run', diff --git a/packages/schemas/src/operations/workflow/runFailOp.ts b/packages/schemas/src/operations/workflow/runFailOp.ts new file mode 100644 index 00000000..d7fe5e80 --- /dev/null +++ b/packages/schemas/src/operations/workflow/runFailOp.ts @@ -0,0 +1,24 @@ +import { z } from 'zod'; + +// --- workflow.run.fail --- + +/** + * The most a skill's stated reason carries: a check's summary with the end of + * what it printed, which is what a failure is read for. + */ +export const WORKFLOW_RUN_FAIL_REASON_MAX_CHARS = 20_000; + +export const WorkflowRunFailInputSchema = z.object({ + reason: z + .string() + .min(1) + .max(WORKFLOW_RUN_FAIL_REASON_MAX_CHARS) + .describe( + 'Why the run stops here, as whoever reads the failure needs it — usually bound from the output of the task whose result the skill decided to stop on.', + ), +}); +export type WorkflowRunFailInput = z.infer; + +/** Never produced: the step always fails, with as its message. */ +export const WorkflowRunFailOutputSchema = z.object({}).strict(); +export type WorkflowRunFailOutput = z.infer; diff --git a/packages/schemas/src/runtime/parentInstructions.ts b/packages/schemas/src/runtime/parentInstructions.ts index 93ebfcac..adcfe0af 100644 --- a/packages/schemas/src/runtime/parentInstructions.ts +++ b/packages/schemas/src/runtime/parentInstructions.ts @@ -118,10 +118,13 @@ export type WorkflowRunMetadata = z.infer; /** * The deepest a chain of task-started runs may reach, counting the run at its * head as 1. A publication and the review it starts of its own commit are two; - * nothing a task starts needs a third, and without a bound a workflow that - * starts itself would never stop. + * a publication that decides on a fix round starts the commission and its own + * republication at two, and that republication's review is the third. Without + * a bound a workflow that starts itself would never stop; with this one fix + * rounds stop after two, since the run the second one republishes is at three + * and can start neither a review nor a commission of its own. */ -export const MAX_WORKFLOW_RUN_DEPTH = 2; +export const MAX_WORKFLOW_RUN_DEPTH = 3; export function workflowRunDepth(metadata: unknown): number { const parsed = WorkflowRunMetadataSchema.safeParse(metadata ?? {}); diff --git a/scripts/decisions-measure-loop.ts b/scripts/decisions-measure-loop.ts new file mode 100644 index 00000000..3b3e8288 --- /dev/null +++ b/scripts/decisions-measure-loop.ts @@ -0,0 +1,473 @@ +/** + * Plan 316 §7 — how well the loop's own decisions can be made without a + * reasoning model, measured on the loop's own history before Publish Local + * Changes makes them for itself. + * + * yarn decisions:measure-loop collect --space [--repo aflowai/aflow] [--prs 91-124] + * yarn decisions:measure-loop measure [--labels ] [--target 0.9] + * + * `collect` reads every Publish Local Changes and Review Local Changes run of + * the space through the stack's API (the API key in AFLOW_API_KEY, the stack + * at API_URL or http://localhost:3000) and every failing CI job of the pull + * requests named through `gh api`, and writes the labelled set to + * `.debug/decisions/cases.json`: each failed check with what the next + * publication of its branch did, each review finding with whether a later + * publication closed it, each review with whether a fix round followed, each + * failing job's log end with the remedy that followed. A label the records + * cannot give is left out rather than guessed; `--labels` names a file of the + * operator's own (`{ "": "