Prevent premature drafter structured output

.agents/skills/add-pr-reviewer-to-repo/SKILL.md

@@ -119,9 +119,9 @@ jobs:
       contents: read # Read repository files and PR diffs
       pull-requests: write # Post review comments
       issues: write # Create security incident issues if secrets detected
-      checks: write # (Optional) Show review progress as a check run
+      checks: write # Show review progress as a check run
       id-token: write # Required for OIDC authentication to AWS Secrets Manager
-      actions: write # Cache read/write for review-lock deduplication and binary cache
+      actions: write # Best-effort REST cleanup of lock caches and processed feedback artifacts
 ```
 
 All three events (`pull_request`, `issue_comment`, `pull_request_review_comment`) have full OIDC/secret access for same-repo PRs, so the reusable workflow handles everything directly.
@@ -208,9 +208,9 @@ jobs:
       contents: read # Read repository files and PR diffs
       pull-requests: write # Post review comments
       issues: write # Create security incident issues if secrets detected
-      checks: write # (Optional) Show review progress as a check run
+      checks: write # Show review progress as a check run
       id-token: write # Required for OIDC authentication to AWS Secrets Manager
-      actions: write # Cache read/write for review-lock deduplication and binary cache
+      actions: write # Best-effort REST cleanup of lock caches and processed feedback artifacts
     with:
       trigger-run-id: ${{ github.event_name == 'workflow_run' && format('{0}', github.event.workflow_run.id) || '' }}
 ```
@@ -262,8 +262,7 @@ wiring.
 For repos that already have the workflows, verify each item:
 
 - [ ] **Version/tag is current** — compare the `@VERSION` in `uses:` against the latest release from `gh release list --repo docker/docker-agent-action --limit 1`. Update if behind.
-- [ ] **All required permissions are present** — `contents: read`, `pull-requests: write`, `issues: write`, `id-token: write`, `actions: write`. Missing any of these causes silent failures or OIDC/artifact errors. Note: missing `actions: write` specifically causes a 403 when the reusable workflow tries to store binary cache or upload/download artifacts (cache write operations require `write`; artifact download requires only `read`).
-- [ ] **`checks: write` is present** (optional but recommended) — without it the review won't appear as a check run on the PR.
+- [ ] **All required permissions are present** — `contents: read`, `pull-requests: write`, `issues: write`, `checks: write`, `id-token: write`, `actions: write`. Missing any of these causes workflow validation or OIDC/API failures. `actions: write` is required for best-effort REST cleanup of review-lock caches and processed feedback artifacts; `actions/cache` restore/save uses the runner cache service and does not depend on this GitHub token scope.
 - [ ] **Bot-filter `if` condition is correct** — the condition must filter out `docker-agent`, `docker-agent[bot]`, any `Bot` user type, and comments containing `<!-- docker-agent-review -->` or `<!-- docker-agent-review-reply -->`. A missing or incomplete filter causes infinite review loops.
 - [ ] **Fork repos: reviewer-target gate is present** — if `pull_request.review_requested` is enabled, `save-context` must run it only when `github.event.requested_reviewer.login == 'docker-agent'`. A request for a human, team, or other bot must leave `save-context` skipped and must not invoke the privileged `workflow_run` handler; a request for exactly `docker-agent` proceeds.
 - [ ] **Fork artifact rollout order is safe** — upgrade the reusable workflow before switching the trigger to the minimized locator artifact. New minimized artifacts with an older reusable workflow are not guaranteed to work; roll back by restoring the legacy artifact format until the consumer is upgraded.
@@ -291,9 +290,9 @@ jobs:
       ...
 ```
 
-### Artifact download fails with 403
+### Reusable workflow permission validation fails
 
-**Cause:** `actions: write` is missing from the `pr-review.yml` job permissions. This permission is required by the reusable workflow for artifact operations on all setups, not just fork repos.
+**Cause:** `actions: write` is missing from the `pr-review.yml` job permissions. The reusable workflow declares this permission for best-effort REST cleanup of review-lock caches and processed feedback artifacts; it is not needed by `actions/cache` restore/save.
 
 **Fix:** Add `actions: write` to the `permissions` block on the `review` job in `pr-review.yml`.
 
@@ -324,7 +323,7 @@ jobs:
 
 **Cause:** `checks: write` permission is absent.
 
-**Fix:** Add `checks: write` to the job `permissions` block. This is optional but strongly recommended so the review progress is visible in the PR's Checks tab.
+**Fix:** Add the required `checks: write` permission to the job `permissions` block.
 
 ---
 
@@ -403,8 +402,8 @@ Check the `permissions:` block on the `review` job in `pr-review.yml`:
 - [ ] `pull-requests: write`
 - [ ] `issues: write`
 - [ ] `id-token: write` ← OIDC; missing this breaks all credential fetching
-- [ ] `checks: write` ← optional but strongly recommended
-- [ ] `actions: write` ← required for all setups (reusable workflow uses it for artifact operations)
+- [ ] `checks: write` ← required for review progress check runs
+- [ ] `actions: write` ← required for best-effort REST cleanup of lock caches and processed feedback artifacts
 
 #### Trigger types
 

AGENTS.md

@@ -19,7 +19,7 @@ Anything else here (workflows under `.github/workflows/`, scripts, tests) exists
 ```
 .
 ├── action.yml                       # ← Root action ("Docker Agent Runner"). Composite. Source of truth for inputs/outputs.
-├── DOCKER_AGENT_VERSION             # Pinned docker-agent version (currently v1.54.0). Read at runtime by action.yml.
+├── DOCKER_AGENT_VERSION             # Pinned docker-agent version (currently v1.140.0). Read at runtime by action.yml.
 ├── package.json                     # pnpm workspace root. Scripts: build, test, lint, format, actionlint.
 ├── tsup.config.ts                   # Bundles src/<name>/index.ts → dist/<name>.js (ESM, Node 24, fully bundled).
 ├── tsconfig.json                    # TS config. rootDir=src, target ES2024, strict.
@@ -186,6 +186,7 @@ The action runs untrusted input (PR titles, bodies, comments, diffs) through an
 
 ### `review-pr` action specifics
 
+- `review-pr/agents/pr-review.yaml` uses config version 16 and sets only the drafter's `structured_output` to `mode: tool`. Tool mode prevents mid-analysis narration from being accepted as the drafter's terminal structured result; the verifier intentionally stays on native structured output. This requires docker-agent >= v1.125.0, which the current v1.140.0 `DOCKER_AGENT_VERSION` pin satisfies. Do not downgrade the config version, switch the drafter back to native mode, or enable tool mode for the verifier without revalidating this contract.
 - Uses a **best-effort cache lock** (`pr-review-lock-<repo>-<pr>-*` cache key) to avoid concurrent reviews on the same PR. Completed runs release the lock by saving a `-released` marker cache entry that shadows their lock entry (cache saves work regardless of token scopes; the REST cache DELETE is best-effort cleanup only). The 3600s TTL is a fallback for crashed holders and must stay above the review agent's 2700s wall-clock budget (45 min, enforced by the root action's `total-timeout` across all attempts) so an in-flight review is never treated as stale. Reviews are idempotent so the small race window is acceptable.
 - **Memory persistence** uses `actions/cache` keyed by `pr-review-memory-<repo>-<job>-<run_id>` with prefix-based restore. The review memory database lives at `${{ github.workspace }}/.cache/pr-review-memory.db`.
 - **Fork workflow-run private context files** are canonicalized from GitHub API data. Trigger artifacts are untrusted locators only; server-derived PR/comment data and an immutable 40-hex SHA drive authorization, prompts, posting, and checkout. Attempt-specific randomized `runner.temp` roots are `0700`; the resolver exclusively creates canonical JSON at `0600`, and a pre-upload guard verifies containment, non-symlink status, and exact modes. Isolated consumers select the same-run artifact by immutable ID, verify its digest, then restore and verify `0700/0600` because artifact modes are not preserved. Canonical-derived files are exclusively created at `0600` in the same private job root. Never use predictable shared `/tmp` paths for locator, canonical, or derived trigger context; unrelated reviewed runtime temporary files are outside this invariant. The artifact name includes the run ID and run attempt to avoid rerun collisions. If the pinned bundle has no resolver, workflow-run routes skip fail-closed while direct routes continue.

review-pr/README.md

@@ -35,9 +35,9 @@ jobs:
       contents: read # Read repository files and PR diffs
       pull-requests: write # Post review comments
       issues: write # Create security incident issues if secrets detected
-      checks: write # (Optional) Show review progress as a check run
+      checks: write # Show review progress as a check run
       id-token: write # Required for OIDC authentication to AWS Secrets Manager
-      actions: write # Cache read/write for review-lock deduplication and binary cache
+      actions: write # Best-effort REST cleanup of lock caches and processed feedback artifacts
 ```
 
 That's it. All three events (`pull_request`, `issue_comment`, `pull_request_review_comment`) have full OIDC/secret access for same-repo PRs, so the reusable workflow handles everything directly.
@@ -116,9 +116,9 @@ jobs:
       contents: read # Read repository files and PR diffs
       pull-requests: write # Post review comments
       issues: write # Create security incident issues if secrets detected
-      checks: write # (Optional) Show review progress as a check run
+      checks: write # Show review progress as a check run
       id-token: write # Required for OIDC authentication to AWS Secrets Manager
-      actions: write # Required by reusable workflow for artifact operations; also needed to download trigger artifacts
+      actions: write # Best-effort REST cleanup of lock caches and processed feedback artifacts
     with:
       trigger-run-id: ${{ github.event_name == 'workflow_run' && format('{0}', github.event.workflow_run.id) || '' }}
 ```
@@ -185,7 +185,7 @@ Adds `synchronize` to also trigger on every push to the PR branch. Opt in if you
 Auto-review only runs on PRs authored by org members. A PR opened by an external or fork contributor is **not** reviewed automatically. To get one reviewed, an org member drives it through GitHub's native UI in two steps:
 
 1. **Approve the workflow run.** For PRs from first-time and external contributors, GitHub holds all Actions runs until a maintainer approves them (governed by the repository's `Settings` → `Actions` → `General` fork-PR approval policy). Click **Approve and run workflows** on the PR; until then nothing runs, including the PR review trigger.
-2. **Request a review from `docker-agent`.** In the PR sidebar, under **Reviewers**, add `docker-agent`. This fires a `review_requested` event and starts the review, shown as a check run (if `checks: write` is granted).
+2. **Request a review from `docker-agent`.** In the PR sidebar, under **Reviewers**, add `docker-agent`. This fires a `review_requested` event and starts the review, shown as a check run.
 
 That is the entire flow. **No special commands or workflow inputs are needed**: not the deprecated `/review` comment, not `workflow_dispatch`, and no caller-side configuration. The review is authorized by the requesting org member rather than the PR author, which is what lets an external contributor's PR be reviewed on demand. The request is safe by construction: GitHub only lets users with triage or write access request a reviewer, and the reusable workflow verifies org membership before any review work runs. An external contributor cannot trigger a review of their own PR.
 
@@ -204,7 +204,7 @@ with:
 | ------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------- |
 | Request review from `docker-agent`   | **Primary trigger.** Add `docker-agent` as a reviewer in the PR sidebar — review starts automatically, shown as a check run. Authorized by the requesting org member, so it also works for external/fork contributors' PRs. |
 | PR opened/ready                      | Auto-reviews when a PR is opened or marked ready for review (org-member-authored PRs).                                                   |
-| ~~`/review`~~ _(deprecated)_         | Re-trigger a review, or trigger manually when auto-review hasn't run (e.g. after a force-push). Shows as a check run if `checks: write` is granted. |
+| ~~`/review`~~ _(deprecated)_         | Re-trigger a review, or trigger manually when auto-review hasn't run (e.g. after a force-push). Shows progress as a check run. |
 | Reply to review comment              | Responds in-thread and captures feedback to improve future reviews.                                                                      |
 | `@docker-agent` mention              | Answers questions and clarifies review findings. Works in both PR-level issue comments and inline file-line review comments, including on fork PRs (via the trigger workflow). |
 

review-pr/agents/pr-review.yaml

@@ -1,7 +1,7 @@
 # Copyright The Docker Agent Action authors
 # SPDX-License-Identifier: Apache-2.0
 
-version: "6"
+version: "16"
 
 models:
   sonnet:
@@ -749,13 +749,14 @@ agents:
       Use `read_file` to read that path — it contains the unified diff you must analyze.
 
       If the orchestrator's message contains a file path, read it FIRST before doing anything
-      else. If the file is not found, return this exact response immediately:
+      else. If the file is not found, immediately make your single `__structured_output__`
+      tool call with exactly these arguments:
       ```json
-      {"findings": [], "summary": "ERROR: Diff file not found at the specified path. The orchestrator must write the diff to disk before delegating."}
+      {"findings": [], "summary": "ERROR: Diff file not found at the specified path. The orchestrator must write the diff to disk before delegating.", "review_complete": false}
       ```
 
       Do NOT guess other file paths or search the filesystem for the diff. Read the ONE path
-      the orchestrator gave you, or return the error above.
+      the orchestrator gave you, or return the error above through `__structured_output__`.
 
       ## Domain-Specific Review Guides
 
@@ -941,7 +942,13 @@ agents:
 
       ## Output
 
-      Return structured JSON (schema-enforced). For each finding: `file` (repo-relative path),
+      Your final response MUST be exactly one standalone `__structured_output__` tool call.
+      Make that call only after you have read and analyzed the diff and finished any permitted
+      source-file checks. Do not emit plain-text status updates, progress narration, analysis,
+      or JSON before the tool call, and do not call `__structured_output__` early. The single
+      tool call is terminal: after making it, emit nothing else.
+
+      In that tool call, provide each finding's `file` (repo-relative path),
       `line` (exact, 1-indexed — see algorithm below), `severity`, `category` (one of:
       security, logic_error, resource_leak, concurrency, error_handling, data_integrity, other),
       `issue` (one-line summary), `details` (trigger + impact), `in_diff` (true if on a `+` line).
@@ -979,6 +986,7 @@ agents:
       Use exact 1-indexed line numbers. Do NOT say "around line X".
 
     structured_output:
+      mode: tool
       name: draft_findings
       description: Bug hypotheses found in the PR diff
       strict: true

src/pr-review-agent/__tests__/pr-review-yaml.test.ts

@@ -109,6 +109,54 @@ function isRefusalContent(value: string): boolean {
 const DESCRIPTION_CONTRACT =
   "the schema only rejects empty (minLength); whitespace-only or the bare literal 'placeholder' is refusal output rejected by instruction";
 
+const MISSING_DIFF_SUMMARY =
+  'ERROR: Diff file not found at the specified path. The orchestrator must write the diff to disk before delegating.';
+
+describe('drafter tool-mode structured output contract', () => {
+  const drafter = normalize(drafterAgent);
+
+  it('uses config version 16 required for tool-mode structured output', () => {
+    expect(source).toMatch(/^version: "16"$/m);
+  });
+
+  it('enables tool mode only for the drafter and leaves the verifier native', () => {
+    expect(drafterAgent).toMatch(/structured_output:\n {6}mode: tool\n {6}name: draft_findings/);
+    expect(verifierAgent).not.toMatch(/structured_output:\n {6}mode: tool/);
+    expect(source.match(/^\s+mode: tool$/gm)).toHaveLength(1);
+  });
+
+  it('requires one terminal standalone output-tool call after diff analysis', () => {
+    expect(drafter).toContain(
+      'Your final response MUST be exactly one standalone `__structured_output__` tool call.',
+    );
+    expect(drafter).toContain(
+      'Make that call only after you have read and analyzed the diff and finished any permitted source-file checks.',
+    );
+    expect(drafter).toContain(
+      'Do not emit plain-text status updates, progress narration, analysis, or JSON before the tool call, and do not call `__structured_output__` early.',
+    );
+    expect(drafter).toContain(
+      'The single tool call is terminal: after making it, emit nothing else.',
+    );
+  });
+
+  it('routes a schema-valid incomplete missing-diff fallback through the output tool', () => {
+    const match = drafterAgent.match(
+      /If the file is not found,[\s\S]*?```json\r?\n\s*(\{[^\r\n]+\})\r?\n\s*```/,
+    );
+    expect(match).not.toBeNull();
+    expect(JSON.parse(match?.[1] ?? '')).toEqual({
+      findings: [],
+      summary: MISSING_DIFF_SUMMARY,
+      review_complete: false,
+    });
+    expect(drafter).toContain(
+      'immediately make your single `__structured_output__` tool call with exactly these arguments',
+    );
+    expect(drafter).toContain('return the error above through `__structured_output__`');
+  });
+});
+
 describe('structured-output schema hardening', () => {
   // Free-text fields and their key indent within each schema block.
   const drafterTextFields: Array<[string, number]> = [