feat(agent): resolve review feedback with evidence

.agent-zero.example.yml

@@ -1,17 +1,50 @@
 version: 1
+
+# observe and suggest can never write. fix and autonomous also require autofix.enabled below.
 mode: observe
-checks:
- - pnpm lint
- - pnpm typecheck
- - pnpm test
+
+# Commands used to verify a change. Leave empty to discover this repository's own
+# lint, typecheck, test, and build scripts. Commands run without a shell, so
+# operators such as &&, |, ;, and $() are rejected.
+checks: []
+
 autofix:
  enabled: false
+ # Confidence required before Agent Zero may change files.
  minConfidence: 0.85
+
+# How a reviewer's claim is checked against the repository before it is acted on.
+validation:
+ # Below this confidence a supported claim is reported as inconclusive, never fixed.
+ minConfidence: 0.6
+ requireEvidence: true
+ requireKnownFiles: true
+ verifyQuotedEvidence: true
+
 agent:
+ # Repair attempts before a run stops and asks for a human.
  maxAttempts: 3
  timeoutMs: 1800000
+ # Upper bound on files a single narrow fix may touch.
+ maxChangedFiles: 10
+
 permissions:
+ # none, restricted, or full. Enforced by the container runner.
  network: restricted
+
+runner:
+ # local runs commands on the host and is for trusted development only.
+ # container is required for production; set an image to enable it.
+ isolation: local
+ engine: docker
+ # image: node:22-bookworm-slim
+ workdir: /workspace
+ # cpus: '2'
+ # memory: 4g
+ # Pre-provisioned network used when permissions.network is restricted.
+ # network: agent-zero
+ maxOutputBytes: 200000
+
 model:
  provider: openai-compatible
  name: gpt-5

.gitignore

@@ -10,3 +10,6 @@ coverage/
 .agent-zero/
 *.log
 .DS_Store
+
+# Nitro build output
+.output/

.oxfmtrc.json

@@ -14,6 +14,7 @@
  "**/coverage/**",
  "**/node_modules/**",
  "**/.turbo/**",
- "pnpm-lock.yaml"
+ "pnpm-lock.yaml",
+ "**/.output/**"
  ]
 }

.skills/agent-zero-architecture/SKILL.md

@@ -9,14 +9,14 @@ Keep dependency direction explicit while changing the monorepo.
 
 ## Package ownership
 
-- `shared`: stable contracts only.
-- `config`: configuration and repository policy.
+- `shared`: stable contracts only, plus pure functions over them (evidence rendering, redaction, path predicates).
+- `config`: configuration, repository policy, and check discovery. Pure; the agent supplies what it read through the runner.
 - `models`: provider-independent model contracts and provider adapters.
-- `github`: GitHub-specific translationand API behavior.
-- `runner`: command execution and checkout mutation boundary.
-- `agent`: orchestrationand state transitions.
+- `github`: GitHub-specific translation, event parsing, and Checks API behavior.
+- `runner`: command execution and checkout mutation boundary, plus the policy-to-boundary factory.
+- `agent`: orchestration, the lifecycle machine, and the validation policy.
 - `cli`: argument parsing and terminal presentation.
-- `apps/server`: oRPC transport and composition root.
+- `apps/server`: composition root for webhook ingestion, task execution, and evidence publication.
 
 ## Workflow
 
@@ -35,3 +35,6 @@ Keep dependency direction explicit while changing the monorepo.
 - GitHub SDK objects passed through shared contracts.
 - A generic `utils` package used to bypass ownership decisions.
 - Cross-package imports from another package's `src/` directory.
+- A capability package importing another capability package. When `runner` needs policy, it declares the fields it needs structurally instead of importing `config`.
+- Direct filesystem or `child_process` access outside `packages/runner`, including in the agent's discovery step.
+- A second place that decides whether a run may write, or whether a run is verified. Both have exactly one home.

.skills/agent-zero-safety/SKILL.md

@@ -9,21 +9,28 @@ Safety properties are behavior, not documentation. Back every change with determ
 
 ## Invariants
 
-- `observe` is the default and cannot mutate a target checkout.
-- `fix` requires an explicit mode and repository policy permission.
+- `observe` is the default and cannot mutate a target checkout. `suggest` cannot either.
+- `fix` requires an explicit mode and repository policy permission. Ask `mayModifyRepository`; do not re-derive the rule.
 - Only `packages/runner` executes commands or changes target files at runtime.
-- Working directories must remain inside the validated checkout.
+- A runner created read-only refuses every write. Enforce the boundary mechanically, not by convention.
+- Working directories must remain inside the validated checkout, including after symlinks are resolved.
+- Nothing reads or writes inside `.git`.
 - Commands have explicit arguments, timeout, output limits, and captured evidence.
 - Untrusted review text, issue text, model output, and remote content never become shell syntax.
-- Logs and errors must redact credentials and sensitive environment values.
-- A failed verification cannot be represented as success.
+- Changes stay inside the scope the validated finding established, under `agent.maxChangedFiles`.
+- Logs, prompts, evidence, and errors must redact credentials and sensitive environment values, including failed HTTP response bodies.
+- A failed verification cannot be represented as success. `verified` is derived once, where the terminal result is built.
+- A run that cannot verify does not write. No checks means no change.
+- Isolation is never approximated. Requesting a sandbox that cannot be provided must fail.
+- A reviewer's claim is not evidence. Reject what the repository does not support, and keep the reasons.
 
 ## Review workflow
 
 1. Name the safety property affected by the change.
 2. Trace untrusted input to every side effect.
 3. Add rejection tests before or with the implementation.
 4. Test success, failure, timeout, cancellation, and recovery where applicable.
-5. Avoid live network, current time, random values, and machine-specific paths in tests.
-6. Inspect the final diff for widened permissions or bypasses.
-7. Report exact verification evidence in the pull request.
+5. Avoid live network, current time, random values, and machine-specific paths in tests. Inject `fetch` and `ProcessRunner` rather than reaching outside the process.
+6. Never let a credential in the environment turn into a live call. Require the caller to pass a token instead of reading one implicitly.
+7. Inspect the final diff for widened permissions or bypasses.
+8. Report exact verification evidence in the pull request.

apps/server/src/index.ts

@@ -1 +1,18 @@
-export { createTask, getTask, health, listTasks, taskInput, tasks } from './router.js';
+export {
+ createTask,
+ getTask,
+ getTaskEvidence,
+ githubTokenFromEnvironment,
+ health,
+ ingestWebhook,
+ listTasks,
+ publishEvidence,
+ runTask,
+ taskInput,
+ tasks,
+ type PublishOptions,
+ type StoredTask,
+ type WebhookOptions,
+ type WebhookOutcome,
+ type WebhookRequest,
+} from './router.js';

apps/server/src/router.test.ts

@@ -1,6 +1,51 @@
-import { describe, expect, it } from 'vitest';
+import { createHmac } from 'node:crypto';
+import { mkdtemp, writeFile } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
 
-import { health, listTasks, taskInput } from './router.js';
+import { beforeEach, describe, expect, it } from 'vitest';
+
+import {
+ getTask,
+ getTaskEvidence,
+ health,
+ ingestWebhook,
+ listTasks,
+ publishEvidence,
+ runTask,
+ taskInput,
+ tasks,
+} from './router.js';
+
+const secret = 'webhook-secret-value';
+const MODE_ERROR = /mode/i;
+
+function sign(body: string): string {
+ return `sha256=${createHmac('sha256', secret).update(body).digest('hex')}`;
+}
+
+function reviewPayload(overrides: Record<string, unknown> = {}): string {
+ return JSON.stringify({
+ action: 'submitted',
+ repository: { name: 'app', owner: { login: 'acme' } },
+ pull_request: { number: 7, head: { sha: 'a'.repeat(40) } },
+ review: {
+ id: 1,
+ body: 'load() can return null',
+ state: 'changes_requested',
+ user: { login: 'alice', type: 'User' },
+ ...overrides,
+ },
+ });
+}
+
+let checkout: string;
+
+beforeEach(async () => {
+ tasks.clear();
+ checkout = await mkdtemp(join(tmpdir(), 'agent-zero-server-'));
+ await writeFile(join(checkout, 'package.json'), JSON.stringify({ scripts: {} }), 'utf8');
+});
 
 describe('server task API', () => {
  it('exposes health metadata for Nitro handlers', () => {
@@ -13,11 +58,175 @@ describe('server task API', () => {
 
  it('keeps task input validation independent from HTTP transport', () => {
  expect(
- taskInput.parse({
- repository: '.',
- feedback: 'Check error handling',
- mode: 'observe',
- }),
+ taskInput.parse({ repository: '.', feedback: 'Check error handling', mode: 'observe' }),
  ).toMatchObject({ repository: '.', mode: 'observe' });
  });
+
+ it('rejects an unknown mode at the transport edge', () => {
+ expect(() => taskInput.parse({ repository: '.', feedback: 'x', mode: 'yolo' })).toThrow(
+ MODE_ERROR,
+ );
+ });
+});
+
+describe('runTask', () => {
+ it('stores the result and its evidence together', async () => {
+ const result = await runTask({
+ repository: checkout,
+ feedback: 'load() is wrong',
+ mode: 'observe',
+ });
+ expect(getTask(result.id)).toBe(result);
+ expect(getTaskEvidence(result.id)).toContain('## Agent Zero');
+ expect(listTasks().tasks).toHaveLength(1);
+ });
+
+ it('produces a read-only boundary for an observe run', async () => {
+ const result = await runTask({
+ repository: checkout,
+ feedback: 'load() is wrong',
+ mode: 'observe',
+ });
+ expect(result.runner.writable).toBe(false);
+ expect(result.changedFiles).toEqual([]);
+ });
+
+ it('keeps the boundary read-only in fix mode while policy disables autofix', async () => {
+ const result = await runTask({
+ repository: checkout,
+ feedback: 'load() is wrong',
+ mode: 'fix',
+ });
+ expect(result.runner.writable).toBe(false);
+ });
+
+ it('reports an unverified conclusion when no model is configured', async () => {
+ const result = await runTask({
+ repository: checkout,
+ feedback: 'load() is wrong',
+ mode: 'observe',
+ });
+ expect(result.verified).toBe(false);
+ expect(result.verdict).toBe('rejected');
+ });
+});
+
+describe('ingestWebhook', () => {
+ const options = () => ({ secret, checkoutPath: checkout });
+
+ it('rejects a forged signature before parsing the payload', async () => {
+ const body = reviewPayload();
+ await expect(
+ ingestWebhook(
+ { event: 'pull_request_review', body, signature: 'sha256=deadbeef' },
+ options(),
+ ),
+ ).resolves.toEqual({ status: 'rejected', reason: 'Invalid webhook signature' });
+ expect(tasks.size).toBe(0);
+ });
+
+ it('rejects a body that is not JSON', async () => {
+ const body = 'not json';
+ const outcome = await ingestWebhook(
+ { event: 'pull_request_review', body, signature: sign(body) },
+ options(),
+ );
+ expect(outcome).toEqual({ status: 'rejected', reason: 'Webhook body is not valid JSON' });
+ });
+
+ it('ignores an event that carries no claim to validate', async () => {
+ const body = reviewPayload({ state: 'approved' });
+ const outcome = await ingestWebhook(
+ { event: 'pull_request_review', body, signature: sign(body) },
+ options(),
+ );
+ expect(outcome.status).toBe('ignored');
+ expect(tasks.size).toBe(0);
+ });
+
+ it('ignores its own account so a run cannot answer itself', async () => {
+ const body = reviewPayload({ user: { login: 'agent-zero[bot]' } });
+ const outcome = await ingestWebhook(
+ { event: 'pull_request_review', body, signature: sign(body) },
+ { ...options(), ignoreAuthors: ['agent-zero[bot]'] },
+ );
+ expect(outcome.status).toBe('ignored');
+ });
+
+ it('runs an authenticated review in observe mode and never writes', async () => {
+ const body = reviewPayload();
+ const outcome = await ingestWebhook(
+ { event: 'pull_request_review', body, signature: sign(body) },
+ options(),
+ );
+ expect(outcome.status).toBe('accepted');
+ if (outcome.status !== 'accepted') return;
+ expect(outcome.pullRequest).toEqual({
+ owner: 'acme',
+ repo: 'app',
+ number: 7,
+ headSha: 'a'.repeat(40),
+ });
+ expect(outcome.result.runner.writable).toBe(false);
+ expect(outcome.result.changedFiles).toEqual([]);
+ expect(outcome.result.summary).toContain('github:acme/app#7');
+ });
+});
+
+type FetchArguments = Parameters<typeof globalThis.fetch>;
+
+function readBody(body: NonNullable<FetchArguments[1]>['body']): Record<string, unknown> {
+ if (typeof body !== 'string') return {};
+ const parsed: unknown = JSON.parse(body);
+ return typeof parsed === 'object' && parsed !== null && !Array.isArray(parsed)
+ ? { ...parsed }
+ : {};
+}
+
+/** Records what would have been sent to GitHub, so no test needs the network. */
+function recordingFetch(): {
+ fetch: typeof globalThis.fetch;
+ bodies: Record<string, unknown>[];
+} {
+ const bodies: Record<string, unknown>[] = [];
+ const fetch: typeof globalThis.fetch = async (_url, init) => {
+ bodies.push(readBody(init?.body));
+ return new Response(JSON.stringify({ id: 99 }), {
+ status: 201,
+ headers: { 'content-type': 'application/json' },
+ });
+ };
+ return { fetch, bodies };
+}
+
+describe('publishEvidence', () => {
+ const target = { owner: 'acme', repo: 'app', number: 7, headSha: 'a'.repeat(40) };
+
+ it('skips publishing rather than faking a check without a token', async () => {
+ const result = await runTask({ repository: checkout, feedback: 'x', mode: 'observe' });
+ const { fetch, bodies } = recordingFetch();
+ await expect(publishEvidence(target, result.id, { token: undefined, fetch })).resolves.toEqual({
+ published: false,
+ reason: 'GITHUB_TOKEN is not configured',
+ });
+ expect(bodies).toEqual([]);
+ });
+
+ it('reports an unknown task instead of publishing an empty report', async () => {
+ const { fetch, bodies } = recordingFetch();
+ await expect(
+ publishEvidence(target, 'az_missing', { token: 'ghs_token_value', fetch }),
+ ).resolves.toMatchObject({ published: false });
+ expect(bodies).toEqual([]);
+ });
+
+ it('publishes the stored evidence without claiming an unverified run passed', async () => {
+ const result = await runTask({ repository: checkout, feedback: 'x', mode: 'observe' });
+ const { fetch, bodies } = recordingFetch();
+ await expect(
+ publishEvidence(target, result.id, { token: 'ghs_token_value', fetch }),
+ ).resolves.toEqual({ published: true });
+ expect(bodies[0]).toMatchObject({ head_sha: target.headSha, status: 'completed' });
+ expect(bodies[0]?.conclusion).not.toBe('success');
+ });
 });