From af9c61d93f0a7a550caf1ef140c0ee98971cb12d Mon Sep 17 00:00:00 2001 From: ayushtr-aws Date: Thu, 13 Aug 2026 21:49:05 -0400 Subject: [PATCH 1/2] feat(agent): declare workflow convergence criteria (#517) --- agent/src/workflow/__init__.py | 4 + agent/src/workflow/models.py | 30 ++++ agent/tests/test_workflow_loader.py | 89 ++++++++++++ agent/workflows/coding/new-task-v1.yaml | 4 + agent/workflows/coding/pr-iteration-v1.yaml | 4 + agent/workflows/coding/pr-review-v1.yaml | 3 + agent/workflows/schema/workflow.schema.json | 135 +++++++++++++++++- contracts/workflow-validation/README.md | 7 +- .../schema-bad-convergence-sensor.json | 81 +++++++++++ .../valid-coding-new-task.json | 9 ++ .../valid-coding-pr-review-readonly.json | 6 + .../valid-knowledge-web-research.json | 6 + docs/design/WORKFLOWS.md | 31 +++- .../content/docs/architecture/Workflows.md | 31 +++- 14 files changed, 430 insertions(+), 10 deletions(-) create mode 100644 contracts/workflow-validation/schema-bad-convergence-sensor.json diff --git a/agent/src/workflow/__init__.py b/agent/src/workflow/__init__.py index 9ffcef994..10d62bd0c 100644 --- a/agent/src/workflow/__init__.py +++ b/agent/src/workflow/__init__.py @@ -24,6 +24,8 @@ ) from .models import ( AgentConfig, + Convergence, + ConvergenceEarlyExit, Hydration, Limits, ModelPreference, @@ -47,6 +49,8 @@ __all__ = [ "STEP_HANDLERS", "AgentConfig", + "Convergence", + "ConvergenceEarlyExit", "Hydration", "Limits", "ModelPreference", diff --git a/agent/src/workflow/models.py b/agent/src/workflow/models.py index 1411a16eb..3c9b7c5ae 100644 --- a/agent/src/workflow/models.py +++ b/agent/src/workflow/models.py @@ -44,6 +44,11 @@ # 2026-06-08). First-party names: s3, comment, s3_and_comment. DeliverTarget = str TerminalOutcome = Literal["pr_url", "review_posted", "artifact", "comment"] +ConvergenceMode = Literal["test_gated", "artifact_delivered", "human_approved", "review_submitted"] +ConvergenceSensor = Literal["verify_build", "verify_lint"] +ConvergenceTerminalOutcome = Literal[ + "pr_opened", "review_published", "artifact_delivered", "human_approved" +] Status = Literal["draft", "validated", "production", "deprecated"] @@ -151,6 +156,30 @@ class TerminalOutcomes(BaseModel): secondary: list[TerminalOutcome] = Field(default_factory=list) +class ConvergenceEarlyExit(BaseModel): + """Explicit exceptions to the workflow's normal convergence path.""" + + model_config = ConfigDict(frozen=True, extra="forbid") + + allow_on_policy_deny: bool = False + + +class Convergence(BaseModel): + """How a harness decides that this workflow is complete. + + The v1 contract is declarative metadata for conformance tooling. Runtime + success inference continues to use the existing step and terminal-outcome + behavior until a later version explicitly adopts this contract. + """ + + model_config = ConfigDict(frozen=True, extra="forbid") + + mode: ConvergenceMode + required_sensors: list[ConvergenceSensor] = Field(default_factory=list) + terminal_outcomes: list[ConvergenceTerminalOutcome] + early_exit: ConvergenceEarlyExit | None = None + + class Limits(BaseModel): """Workflow-level defaults; per-task / per-repo overrides still win.""" @@ -192,6 +221,7 @@ class Workflow(BaseModel): required_inputs: RequiredInputs | None = None steps: list[Step] terminal_outcomes: TerminalOutcomes + convergence: Convergence | None = None limits: Limits | None = None promotion_gate: PromotionGate | None = None status: Status diff --git a/agent/tests/test_workflow_loader.py b/agent/tests/test_workflow_loader.py index 522537cf7..49469a6c2 100644 --- a/agent/tests/test_workflow_loader.py +++ b/agent/tests/test_workflow_loader.py @@ -45,6 +45,12 @@ def _valid_new_task() -> dict: {"kind": "ensure_pr", "name": "open_pr", "strategy": "create"}, ], "terminal_outcomes": {"primary": "pr_url"}, + "convergence": { + "mode": "test_gated", + "required_sensors": ["verify_build"], + "terminal_outcomes": ["pr_opened"], + "early_exit": {"allow_on_policy_deny": True}, + }, "limits": {"max_turns": 100}, "promotion_gate": {"requires": ["tests:agent/new_task"]}, "status": "production", @@ -61,6 +67,12 @@ def test_parses_valid_coding_workflow(self): assert wf.steps[3].gate == "regression_only" assert wf.steps[4].strategy == "create" assert wf.terminal_outcomes.primary == "pr_url" + assert wf.convergence is not None + assert wf.convergence.mode == "test_gated" + assert wf.convergence.required_sensors == ["verify_build"] + assert wf.convergence.terminal_outcomes == ["pr_opened"] + assert wf.convergence.early_exit is not None + assert wf.convergence.early_exit.allow_on_policy_deny is True def test_unknown_top_level_field_rejected(self): body = _valid_new_task() @@ -93,6 +105,65 @@ def test_requires_repo_false_forbids_clone_repo_via_schema(self): with pytest.raises(WorkflowValidationError): parse_workflow(body) + def test_convergence_is_optional_for_existing_workflows(self): + body = _valid_new_task() + del body["convergence"] + assert parse_workflow(body).convergence is None + + @pytest.mark.parametrize( + ("convergence", "error_path"), + [ + ( + { + "mode": "fixed_iterations", + "terminal_outcomes": ["pr_opened"], + }, + "convergence/mode", + ), + ( + { + "mode": "test_gated", + "terminal_outcomes": ["pr_opened"], + }, + "convergence", + ), + ( + { + "mode": "test_gated", + "required_sensors": ["verify_tests"], + "terminal_outcomes": ["pr_opened"], + }, + "convergence/required_sensors/0", + ), + ( + { + "mode": "review_submitted", + "terminal_outcomes": ["review_published"], + "early_exit": {"ignore_failures": True}, + }, + "convergence/early_exit", + ), + ( + { + "mode": "review_submitted", + "terminal_outcomes": ["pr_opened"], + }, + "convergence/terminal_outcomes", + ), + ], + ) + def test_invalid_convergence_rejected_with_clear_path(self, convergence, error_path): + body = _valid_new_task() + body["convergence"] = convergence + with pytest.raises(WorkflowValidationError, match=error_path): + parse_workflow(body) + + def test_required_sensor_must_have_matching_step(self): + body = _valid_new_task() + body["convergence"]["required_sensors"] = ["verify_lint"] + with pytest.raises(WorkflowValidationError, match="steps"): + parse_workflow(body) + class TestRequiresRepoDefault: def test_coding_defaults_true(self): @@ -113,6 +184,10 @@ def test_knowledge_defaults_false(self): {"kind": "deliver_artifact", "target": "s3_and_comment"}, ] body["terminal_outcomes"] = {"primary": "artifact"} + body["convergence"] = { + "mode": "artifact_delivered", + "terminal_outcomes": ["artifact_delivered"], + } assert parse_workflow(body).resolved_requires_repo is False def test_explicit_value_overrides_domain_default(self): @@ -175,6 +250,20 @@ def test_invalid_yaml_rejected(self, tmp_path: Path): class TestLoadWorkflow: + @pytest.mark.parametrize( + ("workflow_id", "mode", "outcome"), + [ + ("coding/new-task-v1", "test_gated", "pr_opened"), + ("coding/pr-iteration-v1", "test_gated", "pr_opened"), + ("coding/pr-review-v1", "review_submitted", "review_published"), + ], + ) + def test_shipped_coding_workflow_declares_convergence(self, workflow_id, mode, outcome): + workflow = load_workflow(workflow_id) + assert workflow.convergence is not None + assert workflow.convergence.mode == mode + assert workflow.convergence.terminal_outcomes == [outcome] + def test_missing_id_raises(self): with pytest.raises(WorkflowValidationError, match="not found"): load_workflow("coding/does-not-exist-v9") diff --git a/agent/workflows/coding/new-task-v1.yaml b/agent/workflows/coding/new-task-v1.yaml index 7219fdf3e..8832ddd73 100644 --- a/agent/workflows/coding/new-task-v1.yaml +++ b/agent/workflows/coding/new-task-v1.yaml @@ -46,6 +46,10 @@ steps: - { kind: ensure_pr, name: open_pr, strategy: create } terminal_outcomes: primary: pr_url +convergence: + mode: test_gated + required_sensors: [verify_build] + terminal_outcomes: [pr_opened] limits: max_turns: 100 promotion_gate: diff --git a/agent/workflows/coding/pr-iteration-v1.yaml b/agent/workflows/coding/pr-iteration-v1.yaml index f029da240..81a07cef5 100644 --- a/agent/workflows/coding/pr-iteration-v1.yaml +++ b/agent/workflows/coding/pr-iteration-v1.yaml @@ -43,6 +43,10 @@ steps: - { kind: ensure_pr, name: resolve_pr, strategy: push_resolve } terminal_outcomes: primary: pr_url +convergence: + mode: test_gated + required_sensors: [verify_build] + terminal_outcomes: [pr_opened] limits: max_turns: 100 promotion_gate: diff --git a/agent/workflows/coding/pr-review-v1.yaml b/agent/workflows/coding/pr-review-v1.yaml index 486ecfc0b..30d6033fd 100644 --- a/agent/workflows/coding/pr-review-v1.yaml +++ b/agent/workflows/coding/pr-review-v1.yaml @@ -47,6 +47,9 @@ steps: - { kind: ensure_pr, name: resolve_pr, strategy: resolve } terminal_outcomes: primary: pr_url +convergence: + mode: review_submitted + terminal_outcomes: [review_published] limits: max_turns: 100 promotion_gate: diff --git a/agent/workflows/schema/workflow.schema.json b/agent/workflows/schema/workflow.schema.json index b4b753bb4..12ed42373 100644 --- a/agent/workflows/schema/workflow.schema.json +++ b/agent/workflows/schema/workflow.schema.json @@ -2,7 +2,7 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "$id": "https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/agent/workflows/schema/workflow.schema.json", "title": "ABCA Workflow", - "description": "A versioned, declarative description of how the agent executes one kind of task. See docs/design/WORKFLOWS.md and ADR-014. Phase-0 schema for issue #248.", + "description": "A versioned, declarative description of how the agent executes one kind of task. See docs/design/WORKFLOWS.md and ADR-014. Introduced by issue #248; explicit convergence metadata added by issue #517.", "type": "object", "additionalProperties": false, "required": ["id", "version", "domain", "prompt", "hydration", "agent_config", "steps", "terminal_outcomes", "status"], @@ -238,6 +238,91 @@ } } }, + "convergence": { + "type": "object", + "additionalProperties": false, + "required": ["mode", "terminal_outcomes"], + "description": "Declarative contract for how a harness decides the workflow is done. In v1 this documents existing terminal behavior for conformance tooling; it does not change runtime success inference.", + "properties": { + "mode": { + "type": "string", + "description": "The convergence strategy used to decide that the workflow is complete.", + "enum": ["test_gated", "artifact_delivered", "human_approved", "review_submitted"] + }, + "required_sensors": { + "type": "array", + "description": "Verification step results that must be available before convergence can be evaluated.", + "items": { "enum": ["verify_build", "verify_lint"] }, + "uniqueItems": true, + "minItems": 1 + }, + "terminal_outcomes": { + "type": "array", + "description": "Externally observable outcomes that satisfy this convergence contract.", + "items": { + "enum": ["pr_opened", "review_published", "artifact_delivered", "human_approved"] + }, + "uniqueItems": true, + "minItems": 1 + }, + "early_exit": { + "type": "object", + "additionalProperties": false, + "description": "Explicit exceptions that may terminate the workflow before its normal convergence outcome.", + "properties": { + "allow_on_policy_deny": { + "type": "boolean", + "description": "Whether a policy denial may end the workflow without satisfying its normal terminal outcomes.", + "default": false + } + } + } + }, + "allOf": [ + { + "if": { + "required": ["mode"], + "properties": { "mode": { "const": "test_gated" } } + }, + "then": { + "required": ["required_sensors"] + } + }, + { + "if": { + "required": ["mode"], + "properties": { "mode": { "const": "artifact_delivered" } } + }, + "then": { + "properties": { + "terminal_outcomes": { "contains": { "const": "artifact_delivered" } } + } + } + }, + { + "if": { + "required": ["mode"], + "properties": { "mode": { "const": "human_approved" } } + }, + "then": { + "properties": { + "terminal_outcomes": { "contains": { "const": "human_approved" } } + } + } + }, + { + "if": { + "required": ["mode"], + "properties": { "mode": { "const": "review_submitted" } } + }, + "then": { + "properties": { + "terminal_outcomes": { "contains": { "const": "review_published" } } + } + } + } + ] + }, "limits": { "type": "object", "additionalProperties": false, @@ -312,6 +397,54 @@ } } } + }, + { + "if": { + "required": ["convergence"], + "properties": { + "convergence": { + "required": ["required_sensors"], + "properties": { + "required_sensors": { "contains": { "const": "verify_build" } } + } + } + } + }, + "then": { + "properties": { + "steps": { + "contains": { + "type": "object", + "required": ["kind"], + "properties": { "kind": { "const": "verify_build" } } + } + } + } + } + }, + { + "if": { + "required": ["convergence"], + "properties": { + "convergence": { + "required": ["required_sensors"], + "properties": { + "required_sensors": { "contains": { "const": "verify_lint" } } + } + } + } + }, + "then": { + "properties": { + "steps": { + "contains": { + "type": "object", + "required": ["kind"], + "properties": { "kind": { "const": "verify_lint" } } + } + } + } + } } ] } diff --git a/contracts/workflow-validation/README.md b/contracts/workflow-validation/README.md index 9dff51a52..17d271245 100644 --- a/contracts/workflow-validation/README.md +++ b/contracts/workflow-validation/README.md @@ -1,6 +1,7 @@ # Workflow validation parity fixtures -Golden-file test vectors for the workflow cross-field validator (issue #248). +Golden-file test vectors for the workflow schema and cross-field validator +(issues #248 and #517). Each fixture is a workflow file body paired with its **expected verdict** — `valid`, or the exact set of violation codes the validator must report. @@ -51,6 +52,10 @@ here (documented in `validator.py`). Rule 10 (single production per id lineage) is a cross-file/registry property, not a single-file check, so it is also out of scope for this corpus. +Convergence fixtures exercise the optional `convergence` block introduced by +#517. Schema failures cover invalid modes/sensors and required sensors that do +not have a matching verification step. + ## Consumers - **Agent (Python):** [`agent/tests/test_workflow_validation_corpus.py`](../../agent/tests/test_workflow_validation_corpus.py) diff --git a/contracts/workflow-validation/schema-bad-convergence-sensor.json b/contracts/workflow-validation/schema-bad-convergence-sensor.json new file mode 100644 index 000000000..3d4784241 --- /dev/null +++ b/contracts/workflow-validation/schema-bad-convergence-sensor.json @@ -0,0 +1,81 @@ +{ + "name": "schema-bad-convergence-sensor", + "description": "A convergence contract may only require a sensor produced by a matching verification step.", + "workflow": { + "id": "coding/new-task-v1", + "version": "1.0.0", + "domain": "coding", + "requires_repo": true, + "read_only": false, + "prompt": { + "template": "do the thing" + }, + "hydration": { + "sources": [ + "issue", + "task_description" + ] + }, + "agent_config": { + "tier": "standard", + "allowed_tools": [ + "Bash", + "Read", + "Write", + "Edit" + ], + "cedar_policy_modules": [ + "builtin/hard_deny", + "builtin/soft_deny" + ] + }, + "repo_config": { + "provider": "github", + "discover": true + }, + "required_inputs": { + "one_of": [ + "issue_number", + "task_description" + ] + }, + "steps": [ + { + "kind": "clone_repo" + }, + { + "kind": "hydrate_context" + }, + { + "kind": "run_agent" + }, + { + "kind": "verify_build", + "gate": "regression_only" + }, + { + "kind": "ensure_pr", + "strategy": "create" + } + ], + "terminal_outcomes": { + "primary": "pr_url" + }, + "convergence": { + "mode": "test_gated", + "required_sensors": [ + "verify_lint" + ], + "terminal_outcomes": [ + "pr_opened" + ] + }, + "status": "production" + }, + "expected": { + "valid": false, + "violations": [ + "schema" + ] + } +} diff --git a/contracts/workflow-validation/valid-coding-new-task.json b/contracts/workflow-validation/valid-coding-new-task.json index e0bb99e6a..3bc706d3c 100644 --- a/contracts/workflow-validation/valid-coding-new-task.json +++ b/contracts/workflow-validation/valid-coding-new-task.json @@ -71,6 +71,15 @@ "terminal_outcomes": { "primary": "pr_url" }, + "convergence": { + "mode": "test_gated", + "required_sensors": [ + "verify_build" + ], + "terminal_outcomes": [ + "pr_opened" + ] + }, "limits": { "max_turns": 100 }, diff --git a/contracts/workflow-validation/valid-coding-pr-review-readonly.json b/contracts/workflow-validation/valid-coding-pr-review-readonly.json index cc64f9fff..85dc980f4 100644 --- a/contracts/workflow-validation/valid-coding-pr-review-readonly.json +++ b/contracts/workflow-validation/valid-coding-pr-review-readonly.json @@ -61,6 +61,12 @@ "terminal_outcomes": { "primary": "review_posted" }, + "convergence": { + "mode": "review_submitted", + "terminal_outcomes": [ + "review_published" + ] + }, "status": "production" }, "expected": { diff --git a/contracts/workflow-validation/valid-knowledge-web-research.json b/contracts/workflow-validation/valid-knowledge-web-research.json index f040527c7..a11604d1a 100644 --- a/contracts/workflow-validation/valid-knowledge-web-research.json +++ b/contracts/workflow-validation/valid-knowledge-web-research.json @@ -62,6 +62,12 @@ "terminal_outcomes": { "primary": "artifact" }, + "convergence": { + "mode": "artifact_delivered", + "terminal_outcomes": [ + "artifact_delivered" + ] + }, "limits": { "max_turns": 25, "max_budget_usd": 5 diff --git a/docs/design/WORKFLOWS.md b/docs/design/WORKFLOWS.md index aa409b340..f6c59a99e 100644 --- a/docs/design/WORKFLOWS.md +++ b/docs/design/WORKFLOWS.md @@ -100,7 +100,8 @@ A workflow file has the following top-level fields. (Full machine-readable schem | `repo_config` | object | – | How this workflow relates to a **source-control repository**: `{ provider (default github), discover (default true), ignore: [claude_md\|rules\|subagents\|settings\|mcp] }`. `provider` is a VCS abstraction (see [VCS provider abstraction](#vcs-provider-abstraction)); `discover`/`ignore` gate config discovered from the cloned repo (`CLAUDE.md`, `.claude/`, `.mcp.json`). Must be `discover:false` (and `provider` is N/A) when `requires_repo:false`. | | `steps` | Step[] | ✓ | Ordered pipeline phases (see [Step kinds](#step-kinds)). | | `required_inputs` | object | – | Validation contract, e.g. `{ one_of: [issue_number, task_description] }` or `{ all_of: [pr_number] }`. Replaces the scattered required-input checks. | -| `terminal_outcomes` | object | ✓ | What "done" *produces* — `pr_url` \| `review_posted` \| `artifact` \| `comment`. Records the expected artifact; it does **not** override success inference (see [Success inference](#success-inference-and-terminal-outcomes)). | +| `terminal_outcomes` | object | ✓ | What "done" *produces* — `pr_url` \| `review_posted` \| `artifact` \| `comment`. Records the expected artifact; it does **not** override success inference (see [Success inference](#success-inference-terminal-outcomes-and-convergence)). | +| `convergence` | object | – | How a harness decides the task is done: `{ mode, required_sensors?, terminal_outcomes, early_exit? }`. Optional for backward compatibility; validated at workflow load. In v1 this is declarative metadata and does not change runtime finalization. | | `limits` | object | – | `{ max_turns, max_budget_usd }` defaults (per-task / per-repo still override, per [override precedence](./REPO_ONBOARDING.md#override-precedence)). | | `promotion_gate` | object | – | The check contract a version must pass to reach `production` (see [Promotion is earned, not set](#promotion-is-earned-not-set)). `{ requires: [] }` — pre-#236 a concrete test target (`tests:agent/new_task`); post-#236 an eval id (`eval:web-research-quality`). Optional until #236; absent ⇒ test-tier fallback. | | `status` | enum | ✓ | `draft` \| `validated` \| `production` \| `deprecated`. Only `production` resolves for normal tasks. | @@ -126,6 +127,21 @@ Each step declares `on_failure: fail | continue | skip_remaining` (default `fail **The `gate` field (`verify_build` / `verify_lint`).** A verify step declares how its result affects the task verdict: `strict` (any failure gates), `regression_only` (gates only when the check was passing before the agent ran and fails after — the default when unset, matching the legacy pipeline behavior), or `informational` (never gates). A `read_only` workflow never gates regardless of `gate`. The semantics live in exactly one place — `gate_status` in `agent/src/workflow/runner.py` — used by both lanes (#301): the repo-less lane through the runner's `verify_*` step handlers, and the coding lane through the inline post-hook resolution (`pipeline._apply_post_hook_gates`), which consults each declared step's `gate` and `on_failure` (`continue`/`skip_remaining` steps are advisory for the verdict, matching the runner). On the coding lane an *undeclared* `verify_lint` never gates (the legacy behavior — lint is advisory unless a workflow opts in by declaring the step), and the inline ordering is preserved: `ensure_pr` still runs after a gating verify failure so the agent's work surfaces as a reviewable PR even when the task is marked failed. Routing the coding post-hooks bodily through the runner's step handlers (which would stop *before* `ensure_pr` on a gating failure) is the broader runner unification deferred out of #301's scope. +### Convergence contract + +`convergence` makes the workflow's stopping contract explicit for conformance tests and future harnesses. The loader validates the block before the step runner receives the workflow: + +| Field | Required | Meaning | +|---|---|---| +| `mode` | ✓ | `test_gated` \| `artifact_delivered` \| `human_approved` \| `review_submitted`. Non-test modes require their matching terminal outcome. | +| `required_sensors` | For `test_gated` | Verification results that must be evaluated using their step's existing `gate` semantics. Current sensors are `verify_build` and `verify_lint`; each must have a matching step. | +| `terminal_outcomes` | ✓ | Observable convergence signals: `pr_opened` \| `review_published` \| `artifact_delivered` \| `human_approved`. At least one is required. | +| `early_exit.allow_on_policy_deny` | – | Declares that a policy denial may terminate without the normal terminal outcome. Defaults to `false`. | + +This field is distinct from the required top-level `terminal_outcomes` object. The top-level object identifies the result artifact consumed by existing finalization (`pr_url`, `review_posted`, `artifact`, or `comment`). `convergence.terminal_outcomes` names the external signal a harness should observe before considering the workflow complete. For example, a coding workflow can produce a `pr_url` and declare `pr_opened` as its convergence signal. + +The v1 implementation is intentionally descriptive: it parses and exposes the contract but does not branch on it in `runner.py`, emit a new event, or replace `_resolve_overall_task_status`. Existing workflows without the optional block retain their current behavior. + ### Example: shipped coding workflow (`new_task`) ```yaml @@ -156,6 +172,10 @@ steps: - { kind: verify_build, name: build, gate: regression_only } - { kind: ensure_pr, name: open_pr, strategy: create } terminal_outcomes: { primary: pr_url } +convergence: + mode: test_gated + required_sensors: [verify_build] + terminal_outcomes: [pr_opened] limits: { max_turns: 100 } promotion_gate: { requires: [tests:agent/new_task] } # concrete test target; becomes eval:new_task once #236 lands status: production @@ -190,6 +210,9 @@ steps: - { kind: run_agent, name: research } - { kind: deliver_artifact, name: deliver, target: s3_and_comment } terminal_outcomes: { primary: artifact } +convergence: + mode: artifact_delivered + terminal_outcomes: [artifact_delivered] limits: { max_turns: 25, max_budget_usd: 5 } promotion_gate: { requires: [eval:web-research-quality] } # min-sources / citation-quality eval status: production @@ -451,15 +474,15 @@ The gate verifies the workflow does the *right* thing — not that it reproduces Where a migration deliberately changes behavior, the gate's expected output is updated alongside the change as a recorded decision. New non-coding workflows declare their own check (e.g. `web-research` → a minimum-sources / citation-quality eval once #236 exists). -## Success inference and terminal outcomes +## Success inference, terminal outcomes, and convergence -`terminal_outcomes` declares what a workflow is *expected to produce*; it does not replace the agent's deliberately-defensive success model. Today `_resolve_overall_task_status` (`pipeline.py`) keys success off the agent SDK result status plus the build gate, and explicitly refuses to infer success from PR/build presence when the SDK never emitted a `ResultMessage` (so a crashed agent that happens to have left a branch is not reported `COMPLETED`). That refusal stays. `terminal_outcomes` layers on top as the *artifact* check, not a replacement: +Top-level `terminal_outcomes` declares what a workflow is *expected to produce*. `convergence` separately declares which sensor and external signals define completion for conformance tooling. Neither replaces the agent's deliberately-defensive success model in v1. Today `_resolve_overall_task_status` (`pipeline.py`) keys success off the agent SDK result status plus the build gate, and explicitly refuses to infer success from PR/build presence when the SDK never emitted a `ResultMessage` (so a crashed agent that happens to have left a branch is not reported `COMPLETED`). That refusal stays. `terminal_outcomes` layers on top as the *artifact* check, not a replacement: - **`pr_url` / `review_posted`** — agent status is authoritative; the terminal outcome is the artifact the orchestrator's existing finalization decision matrix (ORCHESTRATOR.md) already inspects (PR exists? commits?). No change to that matrix. - **`artifact` (repo-less)** — there is no PR/branch to fall back on, so success = agent status `success`/`end_turn` **and** the `deliver_artifact` step recorded a delivered artifact (S3 key present). If the agent reports success but no artifact was delivered, the task is `FAILED` (nothing produced) — the repo-less analog of "success, no commits, no PR ⇒ FAILED." - **`comment`** — success = agent status success **and** the comment post succeeded. -The point: `terminal_outcomes` makes "what counts as done" declarative *per workflow* without weakening the existing guard against false-positive completion. +The point: `convergence` now documents "what counts as done" per workflow without weakening or silently changing the existing guard against false-positive completion. A later runtime adoption can consume the same declaration explicitly rather than infer policy from workflow IDs. ## Observability & metadata diff --git a/docs/src/content/docs/architecture/Workflows.md b/docs/src/content/docs/architecture/Workflows.md index a05ee80c4..9bb2357e5 100644 --- a/docs/src/content/docs/architecture/Workflows.md +++ b/docs/src/content/docs/architecture/Workflows.md @@ -104,7 +104,8 @@ A workflow file has the following top-level fields. (Full machine-readable schem | `repo_config` | object | – | How this workflow relates to a **source-control repository**: `{ provider (default github), discover (default true), ignore: [claude_md\|rules\|subagents\|settings\|mcp] }`. `provider` is a VCS abstraction (see [VCS provider abstraction](#vcs-provider-abstraction)); `discover`/`ignore` gate config discovered from the cloned repo (`CLAUDE.md`, `.claude/`, `.mcp.json`). Must be `discover:false` (and `provider` is N/A) when `requires_repo:false`. | | `steps` | Step[] | ✓ | Ordered pipeline phases (see [Step kinds](#step-kinds)). | | `required_inputs` | object | – | Validation contract, e.g. `{ one_of: [issue_number, task_description] }` or `{ all_of: [pr_number] }`. Replaces the scattered required-input checks. | -| `terminal_outcomes` | object | ✓ | What "done" *produces* — `pr_url` \| `review_posted` \| `artifact` \| `comment`. Records the expected artifact; it does **not** override success inference (see [Success inference](#success-inference-and-terminal-outcomes)). | +| `terminal_outcomes` | object | ✓ | What "done" *produces* — `pr_url` \| `review_posted` \| `artifact` \| `comment`. Records the expected artifact; it does **not** override success inference (see [Success inference](#success-inference-terminal-outcomes-and-convergence)). | +| `convergence` | object | – | How a harness decides the task is done: `{ mode, required_sensors?, terminal_outcomes, early_exit? }`. Optional for backward compatibility; validated at workflow load. In v1 this is declarative metadata and does not change runtime finalization. | | `limits` | object | – | `{ max_turns, max_budget_usd }` defaults (per-task / per-repo still override, per [override precedence](/sample-autonomous-cloud-coding-agents/architecture/repo-onboarding#override-precedence)). | | `promotion_gate` | object | – | The check contract a version must pass to reach `production` (see [Promotion is earned, not set](#promotion-is-earned-not-set)). `{ requires: [] }` — pre-#236 a concrete test target (`tests:agent/new_task`); post-#236 an eval id (`eval:web-research-quality`). Optional until #236; absent ⇒ test-tier fallback. | | `status` | enum | ✓ | `draft` \| `validated` \| `production` \| `deprecated`. Only `production` resolves for normal tasks. | @@ -130,6 +131,21 @@ Each step declares `on_failure: fail | continue | skip_remaining` (default `fail **The `gate` field (`verify_build` / `verify_lint`).** A verify step declares how its result affects the task verdict: `strict` (any failure gates), `regression_only` (gates only when the check was passing before the agent ran and fails after — the default when unset, matching the legacy pipeline behavior), or `informational` (never gates). A `read_only` workflow never gates regardless of `gate`. The semantics live in exactly one place — `gate_status` in `agent/src/workflow/runner.py` — used by both lanes (#301): the repo-less lane through the runner's `verify_*` step handlers, and the coding lane through the inline post-hook resolution (`pipeline._apply_post_hook_gates`), which consults each declared step's `gate` and `on_failure` (`continue`/`skip_remaining` steps are advisory for the verdict, matching the runner). On the coding lane an *undeclared* `verify_lint` never gates (the legacy behavior — lint is advisory unless a workflow opts in by declaring the step), and the inline ordering is preserved: `ensure_pr` still runs after a gating verify failure so the agent's work surfaces as a reviewable PR even when the task is marked failed. Routing the coding post-hooks bodily through the runner's step handlers (which would stop *before* `ensure_pr` on a gating failure) is the broader runner unification deferred out of #301's scope. +### Convergence contract + +`convergence` makes the workflow's stopping contract explicit for conformance tests and future harnesses. The loader validates the block before the step runner receives the workflow: + +| Field | Required | Meaning | +|---|---|---| +| `mode` | ✓ | `test_gated` \| `artifact_delivered` \| `human_approved` \| `review_submitted`. Non-test modes require their matching terminal outcome. | +| `required_sensors` | For `test_gated` | Verification results that must be evaluated using their step's existing `gate` semantics. Current sensors are `verify_build` and `verify_lint`; each must have a matching step. | +| `terminal_outcomes` | ✓ | Observable convergence signals: `pr_opened` \| `review_published` \| `artifact_delivered` \| `human_approved`. At least one is required. | +| `early_exit.allow_on_policy_deny` | – | Declares that a policy denial may terminate without the normal terminal outcome. Defaults to `false`. | + +This field is distinct from the required top-level `terminal_outcomes` object. The top-level object identifies the result artifact consumed by existing finalization (`pr_url`, `review_posted`, `artifact`, or `comment`). `convergence.terminal_outcomes` names the external signal a harness should observe before considering the workflow complete. For example, a coding workflow can produce a `pr_url` and declare `pr_opened` as its convergence signal. + +The v1 implementation is intentionally descriptive: it parses and exposes the contract but does not branch on it in `runner.py`, emit a new event, or replace `_resolve_overall_task_status`. Existing workflows without the optional block retain their current behavior. + ### Example: shipped coding workflow (`new_task`) ```yaml @@ -160,6 +176,10 @@ steps: - { kind: verify_build, name: build, gate: regression_only } - { kind: ensure_pr, name: open_pr, strategy: create } terminal_outcomes: { primary: pr_url } +convergence: + mode: test_gated + required_sensors: [verify_build] + terminal_outcomes: [pr_opened] limits: { max_turns: 100 } promotion_gate: { requires: [tests:agent/new_task] } # concrete test target; becomes eval:new_task once #236 lands status: production @@ -194,6 +214,9 @@ steps: - { kind: run_agent, name: research } - { kind: deliver_artifact, name: deliver, target: s3_and_comment } terminal_outcomes: { primary: artifact } +convergence: + mode: artifact_delivered + terminal_outcomes: [artifact_delivered] limits: { max_turns: 25, max_budget_usd: 5 } promotion_gate: { requires: [eval:web-research-quality] } # min-sources / citation-quality eval status: production @@ -455,15 +478,15 @@ The gate verifies the workflow does the *right* thing — not that it reproduces Where a migration deliberately changes behavior, the gate's expected output is updated alongside the change as a recorded decision. New non-coding workflows declare their own check (e.g. `web-research` → a minimum-sources / citation-quality eval once #236 exists). -## Success inference and terminal outcomes +## Success inference, terminal outcomes, and convergence -`terminal_outcomes` declares what a workflow is *expected to produce*; it does not replace the agent's deliberately-defensive success model. Today `_resolve_overall_task_status` (`pipeline.py`) keys success off the agent SDK result status plus the build gate, and explicitly refuses to infer success from PR/build presence when the SDK never emitted a `ResultMessage` (so a crashed agent that happens to have left a branch is not reported `COMPLETED`). That refusal stays. `terminal_outcomes` layers on top as the *artifact* check, not a replacement: +Top-level `terminal_outcomes` declares what a workflow is *expected to produce*. `convergence` separately declares which sensor and external signals define completion for conformance tooling. Neither replaces the agent's deliberately-defensive success model in v1. Today `_resolve_overall_task_status` (`pipeline.py`) keys success off the agent SDK result status plus the build gate, and explicitly refuses to infer success from PR/build presence when the SDK never emitted a `ResultMessage` (so a crashed agent that happens to have left a branch is not reported `COMPLETED`). That refusal stays. `terminal_outcomes` layers on top as the *artifact* check, not a replacement: - **`pr_url` / `review_posted`** — agent status is authoritative; the terminal outcome is the artifact the orchestrator's existing finalization decision matrix (ORCHESTRATOR.md) already inspects (PR exists? commits?). No change to that matrix. - **`artifact` (repo-less)** — there is no PR/branch to fall back on, so success = agent status `success`/`end_turn` **and** the `deliver_artifact` step recorded a delivered artifact (S3 key present). If the agent reports success but no artifact was delivered, the task is `FAILED` (nothing produced) — the repo-less analog of "success, no commits, no PR ⇒ FAILED." - **`comment`** — success = agent status success **and** the comment post succeeded. -The point: `terminal_outcomes` makes "what counts as done" declarative *per workflow* without weakening the existing guard against false-positive completion. +The point: `convergence` now documents "what counts as done" per workflow without weakening or silently changing the existing guard against false-positive completion. A later runtime adoption can consume the same declaration explicitly rather than infer policy from workflow IDs. ## Observability & metadata From d346c440f88f74c139dc8d428856a1ce3dd2103c Mon Sep 17 00:00:00 2001 From: ayushtr-aws Date: Thu, 13 Aug 2026 22:14:40 -0400 Subject: [PATCH 2/2] fix(agent): address convergence review feedback --- agent/src/workflow/models.py | 4 +- agent/tests/test_workflow_loader.py | 73 +++++++++++++++++-- docs/design/WORKFLOWS.md | 2 +- .../content/docs/architecture/Workflows.md | 2 +- 4 files changed, 72 insertions(+), 9 deletions(-) diff --git a/agent/src/workflow/models.py b/agent/src/workflow/models.py index 3c9b7c5ae..5a91003c2 100644 --- a/agent/src/workflow/models.py +++ b/agent/src/workflow/models.py @@ -46,6 +46,8 @@ TerminalOutcome = Literal["pr_url", "review_posted", "artifact", "comment"] ConvergenceMode = Literal["test_gated", "artifact_delivered", "human_approved", "review_submitted"] ConvergenceSensor = Literal["verify_build", "verify_lint"] +# These are observable signals, unlike artifact-oriented TerminalOutcome values. +# Mode/outcome coupling is enforced by the loader's JSON Schema pass. ConvergenceTerminalOutcome = Literal[ "pr_opened", "review_published", "artifact_delivered", "human_approved" ] @@ -175,7 +177,7 @@ class Convergence(BaseModel): model_config = ConfigDict(frozen=True, extra="forbid") mode: ConvergenceMode - required_sensors: list[ConvergenceSensor] = Field(default_factory=list) + required_sensors: list[ConvergenceSensor] | None = Field(default=None, min_length=1) terminal_outcomes: list[ConvergenceTerminalOutcome] early_exit: ConvergenceEarlyExit | None = None diff --git a/agent/tests/test_workflow_loader.py b/agent/tests/test_workflow_loader.py index 49469a6c2..0ef94ec61 100644 --- a/agent/tests/test_workflow_loader.py +++ b/agent/tests/test_workflow_loader.py @@ -11,8 +11,15 @@ from typing import TYPE_CHECKING import pytest - -from workflow import Workflow, WorkflowValidationError, load_workflow, load_workflow_file +from pydantic import ValidationError + +from workflow import ( + Convergence, + Workflow, + WorkflowValidationError, + load_workflow, + load_workflow_file, +) from workflow.loader import parse_workflow, validate_shape if TYPE_CHECKING: @@ -110,6 +117,34 @@ def test_convergence_is_optional_for_existing_workflows(self): del body["convergence"] assert parse_workflow(body).convergence is None + def test_early_exit_policy_deny_defaults_false(self): + body = _valid_new_task() + body["convergence"]["early_exit"] = {} + workflow = parse_workflow(body) + assert workflow.convergence is not None + assert workflow.convergence.early_exit is not None + assert workflow.convergence.early_exit.allow_on_policy_deny is False + + def test_parses_valid_human_approved_convergence(self): + body = _valid_new_task() + body["convergence"] = { + "mode": "human_approved", + "terminal_outcomes": ["human_approved"], + } + workflow = parse_workflow(body) + assert workflow.convergence is not None + assert workflow.convergence.mode == "human_approved" + assert workflow.convergence.required_sensors is None + assert workflow.convergence.terminal_outcomes == ["human_approved"] + + def test_model_rejects_explicit_empty_required_sensors(self): + with pytest.raises(ValidationError, match="required_sensors"): + Convergence( + mode="review_submitted", + required_sensors=[], + terminal_outcomes=["review_published"], + ) + @pytest.mark.parametrize( ("convergence", "error_path"), [ @@ -127,6 +162,14 @@ def test_convergence_is_optional_for_existing_workflows(self): }, "convergence", ), + ( + { + "mode": "test_gated", + "required_sensors": [], + "terminal_outcomes": ["pr_opened"], + }, + "convergence/required_sensors", + ), ( { "mode": "test_gated", @@ -150,19 +193,37 @@ def test_convergence_is_optional_for_existing_workflows(self): }, "convergence/terminal_outcomes", ), + ( + { + "mode": "artifact_delivered", + "terminal_outcomes": ["pr_opened"], + }, + "convergence/terminal_outcomes", + ), + ( + { + "mode": "human_approved", + "terminal_outcomes": ["pr_opened"], + }, + "convergence/terminal_outcomes", + ), ], ) def test_invalid_convergence_rejected_with_clear_path(self, convergence, error_path): body = _valid_new_task() body["convergence"] = convergence - with pytest.raises(WorkflowValidationError, match=error_path): + with pytest.raises(WorkflowValidationError) as exc: parse_workflow(body) + assert f"{error_path}:" in str(exc.value) - def test_required_sensor_must_have_matching_step(self): + @pytest.mark.parametrize("sensor", ["verify_build", "verify_lint"]) + def test_required_sensor_must_have_matching_step(self, sensor): body = _valid_new_task() - body["convergence"]["required_sensors"] = ["verify_lint"] - with pytest.raises(WorkflowValidationError, match="steps"): + body["convergence"]["required_sensors"] = [sensor] + body["steps"] = [step for step in body["steps"] if step["kind"] != sensor] + with pytest.raises(WorkflowValidationError) as exc: parse_workflow(body) + assert "steps:" in str(exc.value) class TestRequiresRepoDefault: diff --git a/docs/design/WORKFLOWS.md b/docs/design/WORKFLOWS.md index f6c59a99e..1b6e00e98 100644 --- a/docs/design/WORKFLOWS.md +++ b/docs/design/WORKFLOWS.md @@ -134,7 +134,7 @@ Each step declares `on_failure: fail | continue | skip_remaining` (default `fail | Field | Required | Meaning | |---|---|---| | `mode` | ✓ | `test_gated` \| `artifact_delivered` \| `human_approved` \| `review_submitted`. Non-test modes require their matching terminal outcome. | -| `required_sensors` | For `test_gated` | Verification results that must be evaluated using their step's existing `gate` semantics. Current sensors are `verify_build` and `verify_lint`; each must have a matching step. | +| `required_sensors` | For `test_gated` | Verification results a conforming harness would evaluate using their step's existing `gate` semantics. Current sensors are `verify_build` and `verify_lint`; each must have a matching step, enforced at load. | | `terminal_outcomes` | ✓ | Observable convergence signals: `pr_opened` \| `review_published` \| `artifact_delivered` \| `human_approved`. At least one is required. | | `early_exit.allow_on_policy_deny` | – | Declares that a policy denial may terminate without the normal terminal outcome. Defaults to `false`. | diff --git a/docs/src/content/docs/architecture/Workflows.md b/docs/src/content/docs/architecture/Workflows.md index 9bb2357e5..255044f6b 100644 --- a/docs/src/content/docs/architecture/Workflows.md +++ b/docs/src/content/docs/architecture/Workflows.md @@ -138,7 +138,7 @@ Each step declares `on_failure: fail | continue | skip_remaining` (default `fail | Field | Required | Meaning | |---|---|---| | `mode` | ✓ | `test_gated` \| `artifact_delivered` \| `human_approved` \| `review_submitted`. Non-test modes require their matching terminal outcome. | -| `required_sensors` | For `test_gated` | Verification results that must be evaluated using their step's existing `gate` semantics. Current sensors are `verify_build` and `verify_lint`; each must have a matching step. | +| `required_sensors` | For `test_gated` | Verification results a conforming harness would evaluate using their step's existing `gate` semantics. Current sensors are `verify_build` and `verify_lint`; each must have a matching step, enforced at load. | | `terminal_outcomes` | ✓ | Observable convergence signals: `pr_opened` \| `review_published` \| `artifact_delivered` \| `human_approved`. At least one is required. | | `early_exit.allow_on_policy_deny` | – | Declares that a policy denial may terminate without the normal terminal outcome. Defaults to `false`. |