feat(agent): declare workflow convergence criteria

[ayushtr-aws]

Adds an optional, validated convergence contract so workflows can explicitly describe the sensors and terminal signals that define completion. The v1 contract is declarative and preserves existing runtime success behavior.

Area

• cdk — infrastructure, handlers, constructs
• agent — Python runtime / Docker image
• cli — bgagent client
• docs — guides or design sources (docs/guides/, docs/design/)
• tooling — root mise.toml, scripts, CI workflows

Related

Closes #517

Changes

• Add the optional convergence schema and typed workflow models.
• Validate convergence modes, sensors, outcomes, early exits, and matching verification steps at load time.
• Declare convergence for new-task, PR-iteration, and PR-review workflows, with validation corpus coverage.
• Document coding and repo-less convergence examples and synchronize generated documentation.

Validation

• mise run build
• uv run pytest --no-cov tests/test_workflow_loader.py tests/test_workflow_validation_corpus.py tests/test_workflow_tree_valid.py -q (64 passed)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.

[ayushtr-aws]

Self-review — principal-architect pass

Reviewed via the /review_pr workflow (pr-review-toolkit agents: code-reviewer, type-design-analyzer, pr-test-analyzer, comment-analyzer + principal-architect judgment). Posting for transparency since this is a self-authored PR.

Verdict: Approve with nits

Clean, backward-compatible, explicitly declarative-only. The convergence contract is validated at load (JSON schema → Pydantic) and correctly does not touch runtime success inference in v1. No blocking issues. Nits are test-coverage asymmetry and two small doc/comment refinements.

Vision alignment ✅

Serves reviewable outcomes — makes "what counts as done" a declarative, validated per-workflow contract while explicitly preserving the defensive _resolve_overall_task_status guard against false-positive completion. early_exit.allow_on_policy_deny: false is fail-closed. v1-descriptive scoping is the right incremental step.

Blocking issues

None.

Non-blocking suggestions / nits

1. human_approved mode is entirely untested (most substantive gap). It's a first-class ConvergenceMode with its own schema contains rule, but no valid case, fixture, or invalid case exercises it — dropping it from the enum or breaking its outcome-coupling rule would pass all tests. Add a valid mode: human_approved + [human_approved] case and a negative mismatched-outcome case.
2. Cross-field rules tested asymmetrically. Only review_submitted's mode↔outcome rule and only verify_lint's sensor↔step rule have negative tests. The parallel artifact_delivered contains rule and the verify_build→step rule are happy-path only — deleting either schema branch wouldn't fail a test. Add one negative case each.
3. required_sensors default [] is looser than the schema (models.py vs schema minItems: 1). Harmless via the loader (schema runs first), but the model accepts required_sensors=[] while the contract rejects it. Either drop the default or document that [] is the model's spelling of "absent."
4. Docs table cell over-promises runtime behavior. In WORKFLOWS.md (+ mirror), the required_sensors row says results "must be evaluated using their step's gate semantics" — reads as a runtime obligation, contradicting the v1-declarative claim below it. Reword to "results a conforming harness would evaluate … each must have a matching step (enforced at load)."
5. early_exit.allow_on_policy_deny default (false) is never asserted — only true is covered. Add an early_exit: {} case asserting False.
6. Naming trip-hazard. ConvergenceTerminalOutcome (pr_opened/review_published/…) is deliberately disjoint from the existing TerminalOutcome (pr_url/review_posted/…). Docs distinguish them well; a one-line comment on the alias — noting the distinction and that cross-field invariants are enforced by the loader's schema pass, not the model — would help future readers.

Documentation ✅

docs/design/WORKFLOWS.md and the generated Starlight mirror are updated in lockstep and byte-identical (no stale-mirror risk). The terminal_outcomes vs convergence.terminal_outcomes distinction (with the pr_url/pr_opened example) pre-empts the likely misreading. Schema $id description appends #517 provenance without overwriting #248. Only refinement is nit 4.

Tests & CI

• Confirm build (agentcore) goes green before merge (it was still pending at review time; other checks pass). Local mise run build + targeted pytest (57 passed) reported in the PR body.
• Coverage is genuinely additive: happy paths, optionality (backward-compat guard), sensor↔step (lint), JSON-pointer error paths, and all three shipped workflows end-to-end. Gaps are the negative-case asymmetry in nits 1/2/5.
• Bootstrap synth-coverage: N/A — agent-runtime + docs only; no CDK constructs/stacks/CFN resource types touched.

Review agents run

• code-reviewer ✅ — confirmed (conf. 95) no runtime path bypasses schema validation (only parse_workflow calls Workflow.model_validate, after validate_shape); exact enum parity between models.py Literals and the JSON schema.
• type-design-analyzer ✅ — schema-only cross-field validation is sound given the file's single-source-of-truth design; flagged nits 3 and 6.
• pr-test-analyzer ✅ — flagged nits 1, 2, 5 and two loose match patterns.
• comment-analyzer ✅ — docs accurate; flagged nits 4 and 6.
• silent-failure-hunter — omitted: no error-handling/fallback code; validation raises WorkflowValidationError rather than swallowing.
• /security-review — omitted: no IAM/Cedar/network/secrets/input-gateway change; workflow files are first-party trusted config and the change is additive, fail-closed validation.

Human heuristics

• Proportionality ✅ — schema + two thin Pydantic models + 3 YAML declarations; no runtime machinery built ahead of need.
• Coherence ⚠️ minor — the two "terminal outcome" vocabularies are disjoint by design and documented (nit 6).
• Clarity ✅ — names/docstrings bound v1 behavior carefully; fails closed with clear JSON-pointer error paths. One doc over-promise (nit 4).
• Appropriateness ⚠️ minor — tests assert intended behavior but with negative-case gaps (nits 1/2/5); a regression breaking human_approved or the artifact_delivered/verify_build rules would pass today.

Recommendation: Approve pending build (agentcore) green. Nits worth a quick follow-commit — especially human_approved coverage (nit 1) and the "must be evaluated" doc wording (nit 4) — but none block merge.

🤖 Generated with Claude Code

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@25330c7). Learn more about missing BASE report.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #762   +/-   ##
=======================================
  Coverage        ?   92.53%           
=======================================
  Files           ?      340           
  Lines           ?    98376           
  Branches        ?    10874           
=======================================
  Hits            ?    91030           
  Misses          ?     7346           
  Partials        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ayushtr-aws]

Addressed all self-review nits in d346c440:

1. Added valid and mismatched-outcome coverage for human_approved.
2. Added negative coverage for artifact_delivered outcome coupling and both verify_build/verify_lint sensor-to-step rules.
3. Changed omitted required_sensors to None and added min_length=1, with direct model and loader tests rejecting explicit empty lists.
4. Reworded the docs to say a conforming harness would evaluate sensors, preserving the v1 declarative-only scope.
5. Added coverage asserting early_exit.allow_on_policy_deny defaults to False.
6. Added a model comment distinguishing convergence signals from artifact-oriented terminal outcomes and locating cross-field enforcement in the schema pass.

Also tightened schema-path assertions in the convergence validation tests.

Validation:

• mise run build
• Focused workflow suite: 64 passed
• Agent suite: 1601 passed, 82.84% coverage
• CDK suite: 4072 passed

[isadeks]

Tightly-scoped contract addition, and the v1 "descriptive only, no runtime branch" boundary is stated clearly in both the Convergence docstring and WORKFLOWS.md. The parametrized error-path tests asserting exact JSON-pointer paths are the right shape. CI is green (5/5) — this only needs an approval to unblock. Five findings, first two substantive:

1. Two production workflows have no convergence, and the test can't notice. agent/workflows/coding/restack-v1.yaml and agent/workflows/knowledge/web-research-v1.yaml are both status: production but declare no block — 2 of 5 shipped workflows (I verified all five at this head). test_shipped_coding_workflow_declares_convergence (agent/tests/test_workflow_loader.py, ~line 314) parametrizes over exactly the three files this PR updated, so it structurally cannot catch the omission. A test that enumerates the registry and asserts every production workflow declares convergence would either close the gap or make the exclusion an explicit, documented decision. Related: contracts/workflow-validation/valid-knowledge-web-research.json:65 does add a convergence block, so the parity corpus now models a shape production doesn't have.

2. mode: test_gated can be structurally satisfied while semantically vacuous. The new step-matching rules (workflow.schema.json, ~lines 400-453) require only that a step with kind: verify_build/verify_lint exists — not that it can gate. A step with gate: informational never affects the verdict, and per WORKFLOWS.md a read_only: true workflow never gates regardless of gate. So a workflow can declare a test-gated contract whose sensors provably cannot gate anything, and tooling reading the block would be misled.

3. Convergence permits states the JSON Schema rejects (agent/src/workflow/models.py). required_sensors is | None = None with no model-level coupling, so Convergence(mode="test_gated", terminal_outcomes=["pr_opened"]) constructs fine while the equivalent file is rejected; same for the mode↔outcome pairs. The comment above ConvergenceTerminalOutcome acknowledges the loader enforces this, but that leaves two sources of truth and lets programmatic construction produce objects that fail on round-trip. A model_validator(mode="after") would state the invariant once.

4. The parity corpus under-covers what its README claims. contracts/workflow-validation/README.md (~line 55) says schema failures cover "invalid modes/sensors", but the only fixture added is schema-bad-convergence-sensor.json — the missing-step case. Since the corpus is what the Phase-4 registry validator (feat(registry): central agent asset registry for capabilities, skills, plugins, and MCP servers #246) must reproduce verdict-for-verdict, uncovered rules won't be enforced on that second implementation; the Python-side parametrized tests don't travel.

5. comment has no convergence counterpart. TerminalOutcome includes comment with defined success semantics, but ConvergenceTerminalOutcome offers only pr_opened/review_published/artifact_delivered/human_approved — a comment-terminal workflow can't declare a truthful signal.

1, 4 and 5 are cheap; 2 and 3 are design calls I'd defer to you on.

[scottschreckengaust]

Verdict: Approve (with non-blocking nits)

A tightly-scoped, additive, backward-compatible change that makes the workflow stopping contract explicit as declarative metadata. The v1 "descriptive only, no runtime branch" boundary is stated clearly in both the Convergence docstring and WORKFLOWS.md, and I verified it holds: parse_workflow runs validate_shape (JSON Schema) before Workflow.model_validate, and nothing in runner.py/pipeline.py consumes the new block. No blocking issues survived verification.

Governance

• Backing issue #517 carries the approved label (P1, agent-runtime, v1). Branch feat/517-explicit-convergence follows convention; PR Closes #517.
• Acceptance criteria met: schema updated, all three shipped coding workflows declare convergence, invalid config fails at load with clear JSON-pointer paths, design-doc section with a repo-less artifact_delivered example, and no change to default terminal behavior.

Vision alignment

Advances reviewable, bounded outcomes (VISION.md) by making "what counts as done" declarative per workflow without weakening _resolve_overall_task_status's guard against false-positive completion. No blast-radius or cost change; no tenet trade requiring an ADR (ADR-014 already covers workflow-driven tasks). Fail-closed: unknown modes/sensors/outcomes are rejected at load.

Blocking issues

None.

Non-blocking suggestions / nits

1. Two production workflows still declare no convergence, and the shipped-workflow test can't notice. Verified at this head: agent/workflows/coding/restack-v1.yaml, agent/workflows/default/agent-v1.yaml, and agent/workflows/knowledge/web-research-v1.yaml are status: production with no convergence block. test_shipped_coding_workflow_declares_convergence parametrizes over exactly the three files this PR edits, so it structurally cannot flag the omission. The field is optional so this is not a correctness bug, and #517's scope named only the three coding workflows — but a test that enumerates the registry and asserts each production workflow either declares convergence or is an explicit documented exclusion would prevent silent drift.
2. Corpus fixture models a shape the shipped file lacks. contracts/workflow-validation/valid-knowledge-web-research.json now adds an artifact_delivered convergence block, while the real knowledge/web-research-v1.yaml has none. Harmless (fixtures are synthetic vectors) but worth reconciling so the parity corpus reflects production intent.
3. Convergence model permits states the JSON Schema rejects. required_sensors: list[... ] | None = None with no model-level mode-coupling means Convergence(mode="test_gated", terminal_outcomes=["pr_opened"]) constructs fine while the equivalent file is rejected by the schema allOf. Safe today because the loader always runs the schema first and the alias comment documents this, but it leaves two sources of truth; a model_validator(mode="after") would state the invariant once and protect programmatic construction.
4. test_gated can be structurally satisfied yet semantically vacuous. The step-matching allOf rules only require a verify_build/verify_lint step to exist — a step with gate: informational, or any step in a read_only workflow, never gates. A future harness reading the contract could be misled. Acceptable for descriptive v1; note it when a harness starts consuming the block.
5. README over-claims corpus coverage. contracts/workflow-validation/README.md says schema failures cover "invalid modes/sensors", but the only added fixture is schema-bad-convergence-sensor.json (missing-step case). The Python parametrized negative tests don't travel to the Phase-4 registry validator (#246), so consider adding a bad-mode / mismatched-outcome fixture.
6. No comment convergence counterpart. TerminalOutcome includes comment, but ConvergenceTerminalOutcome offers only pr_opened/review_published/artifact_delivered/human_approved; a comment-terminal workflow cannot declare a truthful convergence signal. Fine for the current shipped set; revisit if a comment-terminal workflow ships.

Documentation

docs/design/WORKFLOWS.md and the generated Starlight mirror docs/src/content/docs/architecture/Workflows.md are updated in lockstep — I diffed the new Convergence section and it is byte-identical, so no stale-mirror / "Fail build on mutation" risk. The terminal_outcomes vs convergence.terminal_outcomes distinction (with the pr_url/pr_opened example) pre-empts the likely misreading. Schema $id description appends #517 provenance without dropping #248.

Tests & CI

• CI green (5/5): build (agentcore) pass (15m), Secrets/deps/workflow scan pass, Validate PR title pass, Dead-code advisory pass. Codecov: all modified lines covered.
• Coverage is genuinely additive — happy paths, optionality/backward-compat, sensor↔step (both verify_build and verify_lint now negatively tested), mode↔outcome coupling for review_submitted/artifact_delivered/human_approved, JSON-pointer error paths, model-level min_length=1 rejection, and all three shipped coding workflows end-to-end. Remaining gap is the registry-wide production-coverage assertion (nit 1).
• Bootstrap synth-coverage: N/A — agent-runtime (Python) + docs + contract fixtures only; no CDK constructs/stacks/handlers and no new CloudFormation resource types, so ADR-002 bootstrap bundle updates do not apply.

Review agents run

My subagent execution context cannot spawn nested pr-review-toolkit agents, so I performed the equivalent analysis by hand and independently re-verified the substantive prior-thread findings against this head (d8c957f):

• code-reviewer (hand) — verified only parse_workflow reaches model_validate, always after validate_shape; enum parity between models.py Literals and the JSON Schema enums is exact.
• type-design-analyzer (hand) — confirmed the model/schema divergence (nit 3) is real but loader-gated and documented.
• pr-test-analyzer (hand) — confirmed negative-case coverage now symmetric for sensors and mode↔outcome; the registry-wide gap (nit 1) is genuine and I reproduced it by enumerating shipped workflows.
• comment-analyzer (hand) — docstrings/docs accurate; README over-claim is nit 5.
• silent-failure-hunter — omitted: no error-swallowing/fallback code; validation raises WorkflowValidationError and fails closed.
• /security-review — omitted: no IAM/Cedar/network/secrets/input-gateway change; workflow files are first-party trusted config and the change is additive fail-closed validation.

Human heuristics

• Proportionality ✅ — schema block + two thin frozen Pydantic models + 3 YAML declarations; no runtime machinery built ahead of need.
• Coherence ⚠️ minor — two disjoint "terminal outcome" vocabularies by design, documented and comment-annotated (agent/src/workflow/models.py:35).
• Clarity ✅ — names/docstrings carefully bound v1 scope; clear JSON-pointer error paths.
• Appropriateness ⚠️ minor — tests assert intended behavior but the shipped-workflow test hardcodes three IDs rather than enumerating the registry (nit 1).

Approving; nits 1 and 3 are the most worthwhile follow-ups but none block merge.