Remove /v1/agents/run and use one-step workflows (#55) by jameswnl · Pull Request #56 · jameswnl/lightspeed-stack

jameswnl · 2026-09-29T22:29:50Z

Closes #55.

POST /v1/workflows/run is now the only agent execution endpoint: a one-shot agent invocation is submitted as a one-step workflow using the documented agent/result naming convention with a run-level provider. Intentionally breaking, no compatibility adapter.

Removal

Delete src/app/endpoints/agents.py (manual StepInput construction and get_step_executor dispatch that bypassed workflow middleware, persistence, and transcripts)
Remove AgentRunRequest and its export; document the one-step contract on RunWorkflowRequest instead
Remove the router registration, the agents OpenAPI tag, and the AGENT_RUN action

Dependency

Bump lightspeed-cloud-agents a5b30eb -> ffdc8933 to pick up the LCORE-247: Doc info about run.yaml lightspeed-core/lightspeed-stack#268 merge (unified AgentExecutionSpec normalization shared by one-step and multi-step steps on both runners)

Tests

Unit: drop run_agent_handler tests; add one-step forwarding and provider default/override tests
Integration: new test_workflows_integration.py pins the one-step contract against real cloud-agents (full input coverage, bare-step defaults, one-step/multi-step normalization equality, secret-value rejection)
E2E: drop agents HTTP/handler files; add a migration-example one-step test; Makefile and CI file lists updated

Demos/docs

Demo script agent-* scenarios become oneshot-* one-step workflows (oneshot-ephemeral gains the allowed_skills allowlist its Landlock demo text always described); demo HTML tabs and both cloud-agents design docs updated; openapi.json regenerated

Verification

Unit 3377 passed, integration 264 passed, ruff/black/docstyle clean
pylint/pyright/mypy report only pre-existing hits in untouched files (verified identical on base)
Live-app route check confirms agents/run is gone; demo payloads validated against the real cloud-agents validator

POST /v1/workflows/run is now the only agent execution endpoint: a one-shot agent invocation is submitted as a one-step workflow using the documented agent/result naming convention with a run-level provider. Intentionally breaking with no compatibility adapter. Removal: - Delete src/app/endpoints/agents.py (manual StepInput construction and get_step_executor dispatch that bypassed workflow middleware, persistence, and transcripts). - Remove AgentRunRequest and its export; document the one-step contract on RunWorkflowRequest instead. - Remove the router registration, the agents OpenAPI tag, and the AGENT_RUN action. Dependency: - Bump lightspeed-cloud-agents a5b30eb -> ffdc8933 to pick up the lightspeed-core#268 merge (unified AgentExecutionSpec normalization shared by one-step and multi-step steps on both runners). Tests: - Unit: drop run_agent_handler tests; add one-step forwarding and provider default/override tests to test_workflows_endpoint.py. - Integration: new test_workflows_integration.py pins the one-step contract against real cloud-agents (full input coverage, bare-step defaults, one-step/multi-step normalization equality, secret-value rejection); triage parse test moved over. - E2E: drop agents HTTP/handler files; add a migration-example one-step test to test_workflows_http_e2e.py. Makefile and CI updated. Demos/docs: - Demo script agent-* scenarios become oneshot-* one-step workflows (oneshot-ephemeral gains the allowed_skills allowlist its Landlock demo text always described); demo HTML tabs and both cloud-agents design docs updated; openapi.json regenerated. Verification: unit 3377 passed, integration 264 passed, ruff/black/ docstyle clean, pylint/pyright/mypy report only pre-existing hits in untouched files, live-app route check confirms agents/run is gone.

beesarmy

Found one security/correctness issue: the new secret-rejection test only exercises cloud-agents helpers, not the public stack route. See the inline comment.

beesarmy · 2026-09-29T22:58:00Z

+            }
+        )
+
+        assert validate_definition(definition) != []


This assertion does not cover the /v1/workflows/run boundary. start_workflow_handler passes the raw definition to LocalWorkflowRunner.start, which persists it before build_graph/normalization runs; the route itself never calls validate_definition. Therefore posting this exact credentialed MCP URL can return 202 and store the secret-bearing URL in workflow state before the async step fails. The same applies to a secret-shaped run-level provider.credentials_secret. Please add route-level validation before persistence (and an HTTP regression test), so the documented ‘never reaching run state’ invariant is actually enforced.

jameswnl

Solid PR. Closes #55 cleanly with no compatibility adapter, no secret leakage, and meaningful new contract tests. Local verification: unit + integration tests pass; ruff/black/pydocstyle clean; pyright & pylint only show pre-existing hits in untouched files (verified against harness). CI status checks are green (unit-and-integration 3.12 & 3.13, e2e-mock).

What works well

Pure removal, no adapter. src/app/endpoints/agents.py, AgentRunRequest, Action.AGENT_RUN, the agents OpenAPI tag, and /v1/agents/run are all gone. Verified zero stragglers in src/.
Contract is pinned. test_one_step_matches_multi_step_normalization is the keystone test — it asserts single_normalized[0].model_dump() == multi_normalized[0].model_dump() and locks the "same path/semantics" invariant the issue calls for. Verified by running it.
Security boundary verified. test_secret_value_rejected confirms inline credentialed MCP URLs are rejected by validate_definition and that normalize_definition raises ValueError — checked against the real cloud-agents package.
Demo correctness fix. oneshot_ephemeral now actually threads allowed_skills: ["k8s-diag"], matching what the Landlock demo text has always described.
OpenAPI regeneration is coherent. /v1/agents/run, AgentRunRequest, and the agents tag are removed; RunWorkflowRequest docstring + definition field description now document the one-step convention.
Dep bump is purposeful. a5b30eb → ffdc8933 for the unified AgentExecutionSpec normalization (cloud-agents lightspeed-core#268).

Minor concerns

Untracked doc has stale references. docs/design/cloud-agents/uie-graph-engine-comparison.md:290 still says POST /v1/agents/run dispatches straight to cloud-agents get_step_executor and cites src/app/endpoints/agents.py:8,78 — both false after this PR. The file is currently untracked, so it's not introduced by this PR, but it's part of the working tree and will mislead readers. Either delete that row or include the fix in this PR.
Bare-step spawn default isn't pinned. The issue explicitly requires "explicitly decide whether the canonical default is ephemeral." Cloud-agents currently defaults bare steps to spawn: ephemeral (verified). The new test_bare_one_step_defaults only asserts name == "agent" and output_key == "result" — adding agent_steps[0].spawn == "ephemeral" would lock that decision in and prevent a silent regression.
E2E coverage gap for one-step spawn modes. Acceptance criterion: "none, local, and ephemeral execution are covered by workflow tests." Multi-step workflows have HTTP coverage for all three; one-step workflows only have e2e coverage for spawn=none (test_one_step_workflow_completes). The integration test covers the contract for all three, but it would be cheap to add a spawn=local and a spawn=ephemeral one-step HTTP test for parity.

Non-issues I checked

Pre-existing pyright __wrapped__ / missing provider parameter errors in tests/unit/cloud_agents/test_workflows_endpoint.py are unchanged from harness — same set, same lines.
Pre-existing pylint FieldInfo has no 'xxx' member warnings in src/app/main.py unchanged.
routers.py count went from 30 → 29; test_include_routers and test_check_prefixes were updated to match.

Suggested action

Address (1) — either trim or update the uie-graph-engine-comparison row — since the file is already in the working tree. (2) and (3) are nice-to-haves, not blockers. Otherwise ready to merge.

Address review feedback on PR #56. LocalWorkflowRunner.start persists the raw definition and provider BEFORE build_graph/normalization runs, and the stack route never called validate_definition -- so a secret-bearing definition returned 202 and landed in workflow state before the async step failed. start_workflow_ handler now rejects such input with 422 before dispatch, mirroring cloud-agents' own local/api.py submission gate: - validate_definition errors -> 422 (secret-bearing MCP, malformed steps) - WorkflowDefinition model errors -> 422 (unknown step types/fields) - secret-shaped run-level credentials_secret -> 422 Tests: four handler-level 422 regression tests (each asserting the executor is never started), an HTTP-level 422 rejection test, and spawn=local/ephemeral one-step HTTP tests for spawn-mode parity. Existing handler tests now use a valid one-step definition instead of an empty steps list. Also pins the bare-step ephemeral spawn default.

beesarmy · 2026-09-30T00:18:20Z

+    # runs, so secret-bearing or malformed input must be rejected here --
+    # otherwise it returns 202 and lands in workflow state first. Mirrors
+    # cloud-agents' own local/api.py submission gate.
+    definition_errors = validate_definition(body.definition)


This validation block introduces a new pylint failure: R0914 too-many-locals on start_workflow_handler (16/15). The previous revision only reported the existing E1101 on _get_executor; uv run pylint src/app/endpoints/workflows.py now reports both. Please extract the submission-validation logic into a helper (or otherwise address the count) so the required verify/lint gate remains clean.

beesarmy · 2026-09-30T00:18:49Z

+            status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+            detail={"validation_errors": [str(exc)]},
+        ) from exc
+    run_credentials_secret = provider.get("credentials_secret")


The submission gate still does not validate the run-level provider's name/model. For example, provider={"name": "bogus", "model": "x"} passes this handler, returns 202, and calls executor.start; the cloud-agents normalization later rejects the provider asynchronously, leaving a failed persisted run instead of a 422. Please validate the run-level provider against the executor's approved provider contract at submission time (and add a regression test), while preserving the separate credentials-reference check here.

Address follow-up review feedback on PR #56. - The submission gate now also validates the run-level provider name/model against the executor's approved provider contract (inference_spec_from_provider_config), so unapproved providers 422 instead of persisting a run that fails asynchronously. Regression test: unapproved name rejected, executor never started. - Extract the gate into _validate_workflow_submission, fixing the R0914 too-many-locals on start_workflow_handler; pylint on the file is back to only the pre-existing E1101.

beesarmy

Re-reviewed the latest update. Both follow-up findings are addressed: submission validation is extracted so the new pylint R0914 is gone, and run-level providers are validated before persistence. Focused unit/integration tests pass (30 passed, 1 skipped); targeted pylint now reports only the pre-existing FieldInfo E1101. No blocking findings remain.

beesarmy requested changes Sep 29, 2026

View reviewed changes

jameswnl commented Sep 29, 2026

View reviewed changes

beesarmy reviewed Sep 30, 2026

View reviewed changes

beesarmy approved these changes Sep 30, 2026

View reviewed changes

jameswnl merged commit 70f7480 into harness Sep 30, 2026
3 checks passed

Navigation Menu

Repository navigation

Remove /v1/agents/run and use one-step workflows (#55) - #56

Remove /v1/agents/run and use one-step workflows (#55)#56
jameswnl merged 3 commits into
harnessfrom
issue-55-remove-agents-run

jameswnl commented Sep 29, 2026

Uh oh!

beesarmy left a comment

Uh oh!

beesarmy Sep 29, 2026

Uh oh!

jameswnl left a comment

Uh oh!

beesarmy Sep 30, 2026

Uh oh!

beesarmy Sep 30, 2026

Uh oh!

beesarmy left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jameswnl commented Sep 29, 2026

Removal

Dependency

Tests

Demos/docs

Verification

Uh oh!

beesarmy left a comment

Choose a reason for hiding this comment

Uh oh!

beesarmy Sep 29, 2026

Choose a reason for hiding this comment

Uh oh!

jameswnl left a comment

Choose a reason for hiding this comment

What works well

Minor concerns

Non-issues I checked

Suggested action

Uh oh!

beesarmy Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

beesarmy Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

beesarmy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants