From 3554aa8b170e1d685cb52b12c7c93da3f71ffa8f Mon Sep 17 00:00:00 2001 From: Alexandre Balmes Date: Fri, 29 May 2026 10:38:26 +0200 Subject: [PATCH] feat(api): add pack workflow execution via scope/name URL grammar - `.go-arch-lint.yml`: allow infra-workflowpkg to depend on domain-errors - `CHANGELOG.md`: document F101 route changes, fixes, and breaking change notes - `docs/user-guide/api.md`: update all endpoint examples to (scope, name) two-segment URLs - `docs/user-guide/workflow-packs.md`: add reserved pack names section for local/global/env - `internal/application/service.go`: use ListWithSource for entry population; add Scope/Workflow fields; extract promptFileError helper - `internal/application/service_test.go`: add tests for Scope/Workflow/Source propagation in ListAllWorkflows - `internal/domain/ports/repository.go`: add WorkflowSource, WorkflowInfo types and ListWithSource to WorkflowRepository interface - `internal/domain/ports/repository_test.go`: implement ListWithSource on mock - `internal/domain/workflow/entry.go`: add Scope and Workflow fields with full doc comment - `internal/domain/workflow/entry_test.go`: unit tests for WorkflowEntry field semantics - `internal/infrastructure/repository/composite_repository.go`: migrate ListWithSource return type to ports.WorkflowInfo - `internal/infrastructure/repository/composite_repository_test.go`: update assertions to ports.SourceLocal/SourceGlobal - `internal/infrastructure/repository/yaml_repository.go`: add ListWithSource and WithSource for configurable source reporting - `internal/infrastructure/workflowpkg/discoverer.go`: populate Scope/Workflow on pack entries; add defense-in-depth nameRegex guards - `internal/infrastructure/workflowpkg/discoverer_test.go`: add traversal, invalid name, and scope/workflow field tests - `internal/infrastructure/workflowpkg/manifest.go`: reject reserved scope names (local/global/env) and validate workflow names against nameRegex - `internal/infrastructure/workflowpkg/manifest_test.go`: add reserved scope and invalid workflow name validation tests - `internal/interfaces/api/bridge.go`: expand TrackResumedExecution comment explaining intentional persistence - `internal/interfaces/api/bridge_test.go`: add Scope/Workflow fields to mock entries; add TrackResumedExecution persistence test - `internal/interfaces/api/doc.go`: update route documentation to (scope, name) grammar - `internal/interfaces/api/handlers_executions.go`: use recomposeIdentifier(scope, name) for workflow lookup; update route path - `internal/interfaces/api/handlers_executions_test.go`: add pack/local scope run tests; extract newBlockingExecutionHandlerAPI helper - `internal/interfaces/api/handlers_workflows.go`: pass Scope/Workflow to WorkflowSummary; use recomposeIdentifier; update route paths - `internal/interfaces/api/handlers_workflows_test.go`: add scope/workflow field assertions; update paths to two-segment form; extract newWorkflowHandlerAPI helper - `internal/interfaces/api/routing.go`: add recomposeIdentifier helper to build canonical workflow identifiers from scope+name - `internal/interfaces/api/routing_test.go`: table-driven tests for recomposeIdentifier - `internal/interfaces/api/server_test.go`: update test routes to (scope, name) form - `internal/interfaces/api/types.go`: add Scope and Workflow fields to WorkflowSummary; add Scope to input structs - `internal/interfaces/tui/tab_workflows.go`: fix nil-pointer panic in handleValidate by using fully-qualified entry name - `internal/interfaces/tui/tab_workflows_test.go`: add tests for pack workflow validate path - `internal/testutil/mocks/mocks.go`: implement ListWithSource on mock repository - `tests/fixtures/api/packs/speckit/manifest.yaml`: add speckit pack fixture manifest - `tests/fixtures/api/packs/speckit/state.json`: add speckit pack state fixture - `tests/fixtures/api/packs/speckit/workflows/specify.yaml`: add speckit/specify workflow fixture - `tests/integration/api/functional_test.go`: update route paths to (scope, name) form - `tests/integration/api/pack_workflow_test.go`: integration tests for pack workflow list/get/run/validate via HTTP API - `tests/integration/api/server_integration_test.go`: update route path in integration test Closes #358 --- .go-arch-lint.yml | 1 + CHANGELOG.md | 11 + docs/user-guide/api.md | 214 +++++++++++-- docs/user-guide/workflow-packs.md | 22 ++ internal/application/service.go | 76 ++--- internal/application/service_test.go | 196 ++++++++++++ internal/domain/ports/repository.go | 26 ++ internal/domain/ports/repository_test.go | 9 + internal/domain/workflow/entry.go | 18 +- internal/domain/workflow/entry_test.go | 154 +++++++++ .../repository/composite_repository.go | 13 +- .../repository/composite_repository_test.go | 9 +- .../repository/yaml_repository.go | 35 +- .../infrastructure/workflowpkg/discoverer.go | 21 ++ .../workflowpkg/discoverer_test.go | 146 +++++++++ .../infrastructure/workflowpkg/manifest.go | 13 + .../workflowpkg/manifest_test.go | 183 +++++++++++ internal/interfaces/api/bridge.go | 6 +- internal/interfaces/api/bridge_test.go | 48 ++- internal/interfaces/api/doc.go | 8 +- .../interfaces/api/handlers_executions.go | 11 +- .../api/handlers_executions_test.go | 147 ++++++--- internal/interfaces/api/handlers_workflows.go | 18 +- .../interfaces/api/handlers_workflows_test.go | 133 ++++++-- internal/interfaces/api/routing.go | 40 +++ internal/interfaces/api/routing_test.go | 52 +++ internal/interfaces/api/server_test.go | 11 +- internal/interfaces/api/types.go | 17 +- internal/interfaces/tui/tab_workflows.go | 37 ++- internal/interfaces/tui/tab_workflows_test.go | 96 ++++++ internal/testutil/mocks/mocks.go | 19 ++ .../fixtures/api/packs/speckit/manifest.yaml | 8 + tests/fixtures/api/packs/speckit/state.json | 1 + .../api/packs/speckit/workflows/specify.yaml | 16 + tests/integration/api/functional_test.go | 6 +- tests/integration/api/pack_workflow_test.go | 298 ++++++++++++++++++ .../api/server_integration_test.go | 2 +- 37 files changed, 1922 insertions(+), 199 deletions(-) create mode 100644 internal/domain/workflow/entry_test.go create mode 100644 internal/interfaces/api/routing.go create mode 100644 internal/interfaces/api/routing_test.go create mode 100644 tests/fixtures/api/packs/speckit/manifest.yaml create mode 100644 tests/fixtures/api/packs/speckit/state.json create mode 100644 tests/fixtures/api/packs/speckit/workflows/specify.yaml create mode 100644 tests/integration/api/pack_workflow_test.go diff --git a/.go-arch-lint.yml b/.go-arch-lint.yml index 3e3531a3..b1e23e28 100644 --- a/.go-arch-lint.yml +++ b/.go-arch-lint.yml @@ -549,6 +549,7 @@ deps: infra-workflowpkg: mayDependOn: - domain-workflow + - domain-errors - infra-repository - pkg-registry - pkg-httpx diff --git a/CHANGELOG.md b/CHANGELOG.md index 90f48066..b50d8e80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **F101**: HTTP API workflow routes restructured around a `(scope, name)` two-segment URL grammar to support pack-workflow execution. New routes: `GET /api/workflows/{scope}/{name}`, `POST /api/workflows/{scope}/{name}/run`, `POST /api/workflows/{scope}/{name}/validate`. The `scope` sentinel `local` addresses non-pack workflows (e.g. `/api/workflows/local/deploy-prod/run`); the vendor (pack) name serves as scope for pack workflows (e.g. `/api/workflows/speckit/specify/run`). `WorkflowSummary` gains additive `scope` and `workflow` fields alongside the canonical `name` so clients can build operation URLs directly without splitting on `/`. **Breaking** (F097 not yet released): the previous single-segment routes `/api/workflows/{name}[/run|/validate]` are removed; clients targeting F097 routes must migrate. OpenAPI 3.1 document at `/openapi.json` reflects the new path parameters automatically. SSE event streaming at `/api/executions/{id}/events` is unaffected (keyed by execution UUID). +- **F101**: Pack loader (`internal/infrastructure/workflowpkg/`) now rejects manifests whose `name` matches the reserved scope tokens `local` or `global` with a structured `USER.INPUT.VALIDATION_FAILED` error containing `pack_name` and `reserved_tokens` in the details map. Enforcement is single-source at the manifest validator and applies on install, on discovery listing, and on any future pack-aware code path. Names that merely contain or prefix-share with reserved tokens (`localpack`, `globalpack`, `run`, `validate`) remain valid. + +### Fixed + +- **F101**: Pack workflows are now executable over the HTTP API. F097 advertised pack workflows in `GET /api/workflows` but the single-segment `{name}` placeholder in chi could not match canonical identifiers containing `/` (e.g. `speckit/specify`), so every action endpoint returned `404 Not Found`. The new `(scope, name)` grammar resolves pack identifiers natively and restores parity between `GET /api/workflows` listing and the read/run/validate operations. +- **F101**: TUI `Workflows` tab no longer panics when a user triggers validation on a pack-sourced workflow. Pack entries carry `Name == "packName/workflowName"` while the resolved workflow value object only knows the bare name, so the internal `wfMap` lookup missed and `handleValidate` dereferenced a nil pointer. The validate handler now forwards the fully-qualified entry name (already accepted by the validation service) — same approach used by the existing nil guard in `handleLaunch`. +- **F101**: Workflow pack manifests with traversal-style names are now rejected. `Manifest.Validate` previously applied `^[a-z][a-z0-9-]*$` only to the pack name; entries in the `workflows:` list reached `filepath.Join` unchecked, so a manifest declaring `workflows: ["../../etc/passwd"]` could escape `workflowsDir` to whatever file happened to exist at the resolved path. The same regex is now enforced on every workflow name during validation. The pack discoverer adds defense-in-depth, silently skipping packs and workflow entries whose names fail the regex even when validation is bypassed. + ### Breaking Changes - **F100**: Dedicated `roles/` namespace for agent roles — the environment variable for overriding role search paths is renamed from `AWF_AGENTS_PATH` to `AWF_ROLES_PATH`. All role discovery now happens exclusively under `roles/`-namespaced directories. Migration guide for users upgrading from F098: diff --git a/docs/user-guide/api.md b/docs/user-guide/api.md index b6647cea..da7aab64 100644 --- a/docs/user-guide/api.md +++ b/docs/user-guide/api.md @@ -44,6 +44,12 @@ Once running: ## Endpoints +Workflows are identified by a `(scope, name)` tuple in the URL path: +- **Local workflows**: `scope = "local"`, e.g., `GET /api/workflows/local/deploy-prod` +- **Pack workflows**: `scope = ""`, e.g., `GET /api/workflows/speckit/specify` + +This two-segment grammar replaces the prior single-segment `{name}` placeholder, enabling support for pack workflows which previously resolved with URL mismatches. The tokens `local` and `global` are **reserved scope sentinels** — pack manifests cannot use them as `name`. See [Workflow Packs — Reserved Pack Names](workflow-packs.md#reserved-pack-names). + ### Workflow Discovery & Validation #### List workflows @@ -59,32 +65,79 @@ GET /api/workflows "workflows": [ { "name": "code-review", + "scope": "local", + "workflow": "code-review", "version": "1.0.0", "description": "Review code for bugs and security issues" }, { - "name": "deploy-app", - "version": "2.1.0", - "description": "Deploy application to production" + "name": "speckit/specify", + "scope": "speckit", + "workflow": "specify", + "version": "1.0.0", + "description": "Specification-driven workflow" } ] } } ``` -#### Get workflow details +**Fields:** +- `name` — Canonical workflow identifier (`scope/workflow` for packs, plain name for local) +- `scope` — Scope token (`local` for non-pack, pack name for pack workflows) +- `workflow` — Local part of the workflow name (without scope prefix) +- `version` — Semantic version +- `description` — Brief description + +Clients build operation URLs from the `scope` and `workflow` fields: `GET /api/workflows/{scope}/{workflow}`. + +#### Get workflow details (local) ```http -GET /api/workflows/{name} +GET /api/workflows/local/{name} +``` + +**Example:** +```bash +curl http://localhost:2511/api/workflows/local/deploy-prod ``` **Response (200 OK):** ```json { "body": { - "name": "code-review", + "name": "deploy-prod", "version": "1.0.0", - "description": "Review code for bugs and security issues", + "description": "Deploy application to production", + "states": { + "initial": "build", + "build": { + "type": "step", + "command": "go build ./cmd/app" + } + } + } +} +``` + +#### Get workflow details (pack) + +```http +GET /api/workflows/{pack}/{name} +``` + +**Example:** +```bash +curl http://localhost:2511/api/workflows/speckit/specify +``` + +**Response (200 OK):** +```json +{ + "body": { + "name": "specify", + "version": "1.0.0", + "description": "Specification-driven workflow", "states": { "initial": "read", "read": { @@ -101,14 +154,21 @@ GET /api/workflows/{name} { "status": 404, "title": "Not Found", - "detail": "workflow not found: nonexistent" + "detail": "workflow not found" } ``` -#### Validate workflow +Returned uniformly for: unknown scopes, unknown workflows within a known scope, and unknown packs. + +#### Validate workflow (local) ```http -POST /api/workflows/{name}/validate +POST /api/workflows/local/{name}/validate +``` + +**Example:** +```bash +curl -X POST http://localhost:2511/api/workflows/local/deploy-prod/validate ``` **Response (200 OK — valid workflow):** @@ -120,6 +180,17 @@ POST /api/workflows/{name}/validate } ``` +#### Validate workflow (pack) + +```http +POST /api/workflows/{pack}/{name}/validate +``` + +**Example:** +```bash +curl -X POST http://localhost:2511/api/workflows/speckit/specify/validate +``` + **Response (200 OK — invalid workflow):** ```json { @@ -133,10 +204,10 @@ POST /api/workflows/{name}/validate ### Workflow Execution -#### Run workflow (async) +#### Run workflow (local) ```http -POST /api/workflows/{name}/run +POST /api/workflows/local/{name}/run Content-Type: application/json { @@ -147,6 +218,13 @@ Content-Type: application/json } ``` +**Example:** +```bash +curl -X POST http://localhost:2511/api/workflows/local/code-review/run \ + -H "Content-Type: application/json" \ + -d '{"inputs": {"file": "main.go"}}' +``` + **Response (202 Accepted):** ```json { @@ -159,15 +237,50 @@ Content-Type: application/json The workflow begins execution asynchronously. Use the `execution_id` to monitor progress via the events endpoint or polling. +#### Run workflow (pack) + +```http +POST /api/workflows/{pack}/{name}/run +Content-Type: application/json + +{ + "inputs": { + "feature": "F101" + } +} +``` + +**Example:** +```bash +curl -X POST http://localhost:2511/api/workflows/speckit/specify/run \ + -H "Content-Type: application/json" \ + -d '{"inputs": {"feature": "F101"}}' +``` + +**Response (202 Accepted):** +```json +{ + "body": { + "execution_id": "661f9600-e39c-52e5-c827-557766552000", + "status": "accepted" + } +} +``` + **Error (404 Not Found):** ```json { "status": 404, "title": "Not Found", - "detail": "workflow not found: nonexistent" + "detail": "workflow not found" } ``` +Returned when: +- The scope (pack name or `local`) does not exist +- The workflow name does not exist in that scope +- The pack is not installed + **Error (422 Unprocessable Entity):** ```json { @@ -403,9 +516,10 @@ open http://localhost:2511/docs ### cURL +**Run a local workflow:** ```bash -# Run a workflow -RESULT=$(curl -s -X POST http://localhost:2511/api/workflows/code-review/run \ +# Run a local workflow +RESULT=$(curl -s -X POST http://localhost:2511/api/workflows/local/code-review/run \ -H "Content-Type: application/json" \ -d '{"inputs": {"file": "main.go"}}') @@ -418,11 +532,25 @@ curl -N http://localhost:2511/api/executions/$EXEC_ID/events curl http://localhost:2511/api/executions/$EXEC_ID ``` +**Run a pack workflow:** +```bash +# Run a pack workflow +RESULT=$(curl -s -X POST http://localhost:2511/api/workflows/speckit/specify/run \ + -H "Content-Type: application/json" \ + -d '{"inputs": {"feature": "F101"}}') + +EXEC_ID=$(echo $RESULT | jq -r '.body.execution_id') + +# Stream events +curl -N http://localhost:2511/api/executions/$EXEC_ID/events +``` + ### JavaScript/TypeScript +**Run a local workflow:** ```typescript // Start execution -const response = await fetch('http://localhost:2511/api/workflows/code-review/run', { +const response = await fetch('http://localhost:2511/api/workflows/local/code-review/run', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ inputs: { file: 'main.go' } }) @@ -446,15 +574,29 @@ eventSource.addEventListener('workflow.completed', (event) => { }); ``` +**Run a pack workflow:** +```typescript +// Start pack workflow execution +const response = await fetch('http://localhost:2511/api/workflows/speckit/specify/run', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ inputs: { feature: 'F101' } }) +}); + +const { body } = await response.json(); +const executionId = body.execution_id; +``` + ### Python +**Run a local workflow:** ```python import requests import json # Start execution response = requests.post( - 'http://localhost:2511/api/workflows/code-review/run', + 'http://localhost:2511/api/workflows/local/code-review/run', json={'inputs': {'file': 'main.go'}} ) @@ -472,6 +614,19 @@ for line in response.iter_lines(): print(f'Event: {event_type}') ``` +**Run a pack workflow:** +```python +import requests + +# Start pack workflow execution +response = requests.post( + 'http://localhost:2511/api/workflows/speckit/specify/run', + json={'inputs': {'feature': 'F101'}} +) + +execution_id = response.json()['body']['execution_id'] +``` + ## Error Handling All error responses follow RFC 7807 Problem Details format (provided by Huma): @@ -526,6 +681,7 @@ kill -TERM $(pgrep -f "awf serve") # or Ctrl+C in foreground ### Full workflow execution flow +**Local workflow example:** ```bash # 1. Start server awf serve --port 8080 & @@ -534,10 +690,10 @@ awf serve --port 8080 & curl http://localhost:8080/api/workflows # 3. Validate a workflow before running -curl -X POST http://localhost:8080/api/workflows/code-review/validate +curl -X POST http://localhost:8080/api/workflows/local/code-review/validate # 4. Start a workflow execution -RESPONSE=$(curl -s -X POST http://localhost:8080/api/workflows/code-review/run \ +RESPONSE=$(curl -s -X POST http://localhost:8080/api/workflows/local/code-review/run \ -H "Content-Type: application/json" \ -d '{"inputs": {"file": "src/main.go"}}') @@ -557,6 +713,23 @@ curl "http://localhost:8080/api/history?workflow=code-review&limit=10" curl http://localhost:8080/api/history/stats?workflow=code-review ``` +**Pack workflow example:** +```bash +# 1. Start server with pack installed +awf workflow install myorg/awf-workflow-speckit +awf serve --port 8080 & + +# 2. Run a pack workflow +RESPONSE=$(curl -s -X POST http://localhost:8080/api/workflows/speckit/specify/run \ + -H "Content-Type: application/json" \ + -d '{"inputs": {"feature": "F101"}}') + +EXEC_ID=$(echo $RESPONSE | jq -r '.body.execution_id') + +# 3. Monitor the execution +curl -N http://localhost:8080/api/executions/$EXEC_ID/events +``` + ### Integrate with CI/CD (GitHub Actions) ```yaml @@ -571,7 +744,8 @@ jobs: - name: Run AWF code review via API run: | - RESPONSE=$(curl -s -X POST http://awf-server:2511/api/workflows/code-review/run \ + # Run a local workflow via the API + RESPONSE=$(curl -s -X POST http://awf-server:2511/api/workflows/local/code-review/run \ -H "Content-Type: application/json" \ -d '{"inputs": {"file": "src/main.go"}}') diff --git a/docs/user-guide/workflow-packs.md b/docs/user-guide/workflow-packs.md index 6eb8c459..1417b7dc 100644 --- a/docs/user-guide/workflow-packs.md +++ b/docs/user-guide/workflow-packs.md @@ -177,6 +177,28 @@ awf list Pack workflows appear with `pack/workflow` namespace prefix and `pack` source label. The `awf list` and `awf workflow list` commands show the same packs and versions. +## Reserved Pack Names + +⚠️ **Pack names must not use the reserved scope tokens:** `local` or `global`. + +These tokens are reserved by the HTTP API for URL routing: +- `local` identifies non-pack workflows in URLs like `GET /api/workflows/local/{name}` +- `global` is reserved for future use (global workflow directory feature) + +If you attempt to install or list a pack named `local` or `global`, AWF will reject it with a validation error: + +``` +Error: failed to load pack "local": pack name is reserved — use a different name +``` + +**Examples of allowed pack names:** +- ✅ `speckit` — simple alphanumeric name +- ✅ `my-workflows` — hyphens allowed (snake_case style) +- ✅ `localpack` — `local` as a prefix is fine, only the exact match is reserved +- ✅ `global-lib` — `global` as a prefix is fine + +This protection applies at the pack loader level, so it's enforced uniformly across CLI commands, HTTP API, and any future interfaces. + ## Manifest Format Pack authors create a `manifest.yaml` at the pack root: diff --git a/internal/application/service.go b/internal/application/service.go index 1f298073..b76824d0 100644 --- a/internal/application/service.go +++ b/internal/application/service.go @@ -58,22 +58,28 @@ func (s *WorkflowService) SetPluginOperationProvider(p ports.OperationProvider) // LastValidationWarnings returns the structured ValidationError warnings from the most // recent ValidateWorkflow call. Warnings do not fail validation but are surfaced here -// for callers that want to display or log them (e.g. UNSUPPORTED_PROVIDER — T009 AC-6). +// for callers that want to display or log them (e.g. UNSUPPORTED_PROVIDER). // The slice is replaced on each ValidateWorkflow invocation; nil means no warnings. func (s *WorkflowService) LastValidationWarnings() []workflow.ValidationError { return s.lastValidationWarnings } func (s *WorkflowService) ListAllWorkflows(ctx context.Context) ([]workflow.WorkflowEntry, error) { - names, err := s.repo.List(ctx) + infos, err := s.repo.ListWithSource(ctx) if err != nil { return nil, fmt.Errorf("list workflows: %w", err) } - entries := make([]workflow.WorkflowEntry, 0, len(names)) - for _, name := range names { - entry := workflow.WorkflowEntry{Name: name} - if wf, loadErr := s.repo.Load(ctx, name); loadErr == nil { + entries := make([]workflow.WorkflowEntry, 0, len(infos)) + for _, info := range infos { + src := string(info.Source) + entry := workflow.WorkflowEntry{ + Name: info.Name, + Source: src, + Scope: src, + Workflow: info.Name, + } + if wf, loadErr := s.repo.Load(ctx, info.Name); loadErr == nil { entry.Version = wf.Version entry.Description = wf.Description } @@ -115,9 +121,9 @@ func (s *WorkflowService) GetWorkflow(ctx context.Context, name string) (*workfl } func (s *WorkflowService) ValidateWorkflow(ctx context.Context, name string) error { - wf, err := s.repo.Load(ctx, name) + wf, err := s.GetWorkflow(ctx, name) if err != nil { - return fmt.Errorf("load workflow %s: %w", name, err) + return err } if err := wf.Validate(s.validator.Compile, nil); err != nil { var stateRefErr *workflow.StateReferenceError @@ -152,6 +158,20 @@ func (s *WorkflowService) ValidateWorkflow(ctx context.Context, name string) err return s.validateMCPProxy(wf) } +// promptFileError constructs an ErrorCodeUserInputMissingFile structured error +// with consistent metadata for prompt-file validation failures. +func promptFileError(msg, resolvedPath, stepName string, cause error) error { + return domerrors.NewStructuredError( + domerrors.ErrorCodeUserInputMissingFile, + msg, + map[string]any{ + "path": resolvedPath, + "step": stepName, + }, + cause, + ) +} + func (s *WorkflowService) validatePromptFiles(wf *workflow.Workflow) error { for _, step := range wf.Steps { if step.Type != workflow.StepTypeAgent || step.Agent == nil { @@ -175,49 +195,29 @@ func (s *WorkflowService) validatePromptFiles(wf *workflow.Workflow) error { info, err := os.Stat(path) if err != nil { if os.IsNotExist(err) { - return domerrors.NewStructuredError( - domerrors.ErrorCodeUserInputMissingFile, + return promptFileError( fmt.Sprintf("prompt_file not found: %s", step.Agent.PromptFile), - map[string]any{ - "path": path, - "step": step.Name, - }, - err, + path, step.Name, err, ) } - return domerrors.NewStructuredError( - domerrors.ErrorCodeUserInputMissingFile, + return promptFileError( fmt.Sprintf("prompt_file cannot be accessed: %s", step.Agent.PromptFile), - map[string]any{ - "path": path, - "step": step.Name, - }, - err, + path, step.Name, err, ) } if info.IsDir() { - return domerrors.NewStructuredError( - domerrors.ErrorCodeUserInputMissingFile, + return promptFileError( fmt.Sprintf("prompt_file is a directory, not a file: %s", step.Agent.PromptFile), - map[string]any{ - "path": path, - "step": step.Name, - }, - nil, + path, step.Name, nil, ) } f, err := os.Open(path) if err != nil { - return domerrors.NewStructuredError( - domerrors.ErrorCodeUserInputMissingFile, + return promptFileError( fmt.Sprintf("prompt_file cannot be read: %s", step.Agent.PromptFile), - map[string]any{ - "path": path, - "step": step.Name, - }, - err, + path, step.Name, err, ) } _ = f.Close() @@ -254,7 +254,7 @@ func (s *WorkflowService) validateWithPluginProvider(ctx context.Context, wf *wo // It iterates all steps with mcp_proxy enabled and: // - Emits a WARN log (non-fatal) when the agent provider is codex or opencode. // - Accumulates a structured ValidationError{Level:Warning} for UNSUPPORTED_PROVIDER -// so callers can surface it via LastValidationWarnings() — T009 AC-6. +// so callers can surface it via LastValidationWarnings(). // - Validates plugin_tools[] entries against the injected OperationProvider. // // When opProvider is nil, plugin-level checks are skipped silently. @@ -305,7 +305,7 @@ func (s *WorkflowService) buildKnownPluginSet() map[string]bool { // warnIfUnsupportedProvider emits a WARN log when the step's agent provider operates // the MCP proxy in coexistence mode (codex, copilot, opencode) and mcp_proxy is enabled. // This is non-fatal (warning-only). It also returns a structured ValidationError at warning -// level for the accumulator so callers can surface it via structured output (T009 AC-6). +// level for the accumulator so callers can surface it via structured output. func (s *WorkflowService) warnIfUnsupportedProvider(step *workflow.Step) *workflow.ValidationError { if step.Agent == nil || s.logger == nil { return nil diff --git a/internal/application/service_test.go b/internal/application/service_test.go index 57b7eb0c..5a04afe5 100644 --- a/internal/application/service_test.go +++ b/internal/application/service_test.go @@ -15,6 +15,12 @@ import ( "github.com/stretchr/testify/require" ) +// sourceForName returns SourceLocal for all mock workflows; used by ListWithSource +// implementations in test mocks where source metadata is not relevant. +func sourceForName(_ string) ports.WorkflowSource { + return ports.SourceLocal +} + // Mock implementations type mockRepository struct { workflows map[string]*workflow.Workflow @@ -45,6 +51,18 @@ func (m *mockRepository) List(ctx context.Context) ([]string, error) { return names, nil } +func (m *mockRepository) ListWithSource(ctx context.Context) ([]ports.WorkflowInfo, error) { + names, err := m.List(ctx) + if err != nil { + return nil, err + } + infos := make([]ports.WorkflowInfo, 0, len(names)) + for _, name := range names { + infos = append(infos, ports.WorkflowInfo{Name: name, Source: sourceForName(name)}) + } + return infos, nil +} + func (m *mockRepository) Exists(ctx context.Context, name string) (bool, error) { _, ok := m.workflows[name] return ok, nil @@ -327,6 +345,14 @@ func (m *mockRepo) List(_ context.Context) ([]string, error) { return m.names, nil } +func (m *mockRepo) ListWithSource(_ context.Context) ([]ports.WorkflowInfo, error) { + infos := make([]ports.WorkflowInfo, 0, len(m.names)) + for _, name := range m.names { + infos = append(infos, ports.WorkflowInfo{Name: name, Source: sourceForName(name)}) + } + return infos, nil +} + func (m *mockRepo) Exists(_ context.Context, name string) (bool, error) { _, ok := m.workflows[name] return ok, nil @@ -410,3 +436,173 @@ func TestWorkflowServiceListAllWorkflows_PackErrorDoesNotBlock(t *testing.T) { require.NoError(t, err) assert.Len(t, entries, 1, "pack errors should not block regular workflow listing") } + +// TestWorkflowServiceListAllWorkflows_LocalEntryScopeAndWorkflowFields validates T012 Acceptance Criteria: +// - AC2: ListAllWorkflows populates every local entry with Scope="local", Workflow= +// - AC5: A local entry whose Name contains a slash is NOT split — Scope stays "local" and Workflow stays equal to Name +// - AC6: service_test.go covers happy-path table case asserting Scope, Workflow, Source populated for at least one local entry +func TestWorkflowServiceListAllWorkflows_LocalEntryScopeAndWorkflowFields(t *testing.T) { + tests := []struct { + name string + workflow string + wantName string + }{ + { + name: "simple local workflow", + workflow: "my-workflow", + wantName: "my-workflow", + }, + { + name: "local workflow with slash in name", + workflow: "nested/workflow", + wantName: "nested/workflow", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repo := &mockRepo{ + workflows: map[string]*workflow.Workflow{ + tt.workflow: {Name: tt.workflow, Description: "Test"}, + }, + names: []string{tt.workflow}, + } + svc := application.NewWorkflowService(repo, nil, nil, nil, &noopValidator{}) + + entries, err := svc.ListAllWorkflows(context.Background()) + + require.NoError(t, err) + require.Len(t, entries, 1) + entry := entries[0] + assert.Equal(t, tt.wantName, entry.Name) + assert.Equal(t, "local", entry.Scope, "local entry should have Scope=local") + assert.Equal(t, tt.wantName, entry.Workflow, "local entry Workflow should match Name") + assert.Equal(t, "local", entry.Source, "local entry should have Source=local") + }) + } +} + +// TestWorkflowServiceListAllWorkflows_MixedLocalAndPackEntries validates T012 Acceptance Criteria: +// - AC1-2: Local entries expose Scope="local", Workflow=plain name, Source="local" +// - AC3: Pack entries expose Scope=packName, Workflow=wfName, Name=packName/wfName, Source="pack" +// - AC6: Comprehensive coverage of both local and pack sources in a single result set +func TestWorkflowServiceListAllWorkflows_MixedLocalAndPackEntries(t *testing.T) { + repo := &mockRepo{ + workflows: map[string]*workflow.Workflow{ + "local-wf": {Name: "local-wf", Description: "Local workflow"}, + }, + names: []string{"local-wf"}, + } + packs := &mockPackDiscoverer{ + entries: []workflow.WorkflowEntry{ + {Name: "acme/deploy", Source: "pack", Scope: "acme", Workflow: "deploy", Version: "1.0"}, + }, + } + svc := application.NewWorkflowService(repo, nil, nil, nil, &noopValidator{}) + svc.SetPackDiscoverer(packs) + + entries, err := svc.ListAllWorkflows(context.Background()) + + require.NoError(t, err) + require.Len(t, entries, 2) + + localEntry := entries[0] + assert.Equal(t, "local-wf", localEntry.Name) + assert.Equal(t, "local", localEntry.Scope) + assert.Equal(t, "local-wf", localEntry.Workflow) + assert.Equal(t, "local", localEntry.Source) + + packEntry := entries[1] + assert.Equal(t, "acme/deploy", packEntry.Name) + assert.Equal(t, "acme", packEntry.Scope) + assert.Equal(t, "deploy", packEntry.Workflow) + assert.Equal(t, "pack", packEntry.Source) +} + +// mockRepoWithSources allows tests to specify an explicit source per workflow, +// so the service source-mapping logic can be exercised with non-local origins. +type mockRepoWithSources struct { + workflows map[string]*workflow.Workflow + infos []ports.WorkflowInfo +} + +func (m *mockRepoWithSources) Load(_ context.Context, name string) (*workflow.Workflow, error) { + if wf, ok := m.workflows[name]; ok { + return wf, nil + } + return nil, fmt.Errorf("workflow not found: %s", name) +} + +func (m *mockRepoWithSources) List(_ context.Context) ([]string, error) { + names := make([]string, 0, len(m.infos)) + for _, info := range m.infos { + names = append(names, info.Name) + } + return names, nil +} + +func (m *mockRepoWithSources) ListWithSource(_ context.Context) ([]ports.WorkflowInfo, error) { + return m.infos, nil +} + +func (m *mockRepoWithSources) Exists(_ context.Context, name string) (bool, error) { + _, ok := m.workflows[name] + return ok, nil +} + +// TestWorkflowServiceListAllWorkflows_SourcePropagation validates that +// ListAllWorkflows correctly propagates the Source returned by ListWithSource +// into both WorkflowEntry.Source and WorkflowEntry.Scope. +// This is the regression test for the bug where global/env workflows were +// reported with Source="local" and Scope="local" regardless of their real origin. +func TestWorkflowServiceListAllWorkflows_SourcePropagation(t *testing.T) { + tests := []struct { + name string + source ports.WorkflowSource + wantSource string + wantScope string + }{ + { + name: "local workflow keeps local source", + source: ports.SourceLocal, + wantSource: "local", + wantScope: "local", + }, + { + name: "global workflow reports global source and scope", + source: ports.SourceGlobal, + wantSource: "global", + wantScope: "global", + }, + { + name: "env workflow reports env source and scope", + source: ports.SourceEnv, + wantSource: "env", + wantScope: "env", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repo := &mockRepoWithSources{ + workflows: map[string]*workflow.Workflow{ + "my-workflow": {Name: "my-workflow", Description: "Test"}, + }, + infos: []ports.WorkflowInfo{ + {Name: "my-workflow", Source: tt.source}, + }, + } + svc := application.NewWorkflowService(repo, nil, nil, nil, &noopValidator{}) + + entries, err := svc.ListAllWorkflows(context.Background()) + + require.NoError(t, err) + require.Len(t, entries, 1) + entry := entries[0] + assert.Equal(t, "my-workflow", entry.Name) + assert.Equal(t, tt.wantSource, entry.Source, "Source must match discovery origin") + assert.Equal(t, tt.wantScope, entry.Scope, "Scope must match discovery origin for non-pack entries") + assert.Equal(t, "my-workflow", entry.Workflow) + }) + } +} diff --git a/internal/domain/ports/repository.go b/internal/domain/ports/repository.go index 24e499ca..38baf9bc 100644 --- a/internal/domain/ports/repository.go +++ b/internal/domain/ports/repository.go @@ -6,9 +6,35 @@ import ( "github.com/awf-project/cli/internal/domain/workflow" ) +// WorkflowSource identifies the origin of a discovered workflow. +// It maps directly to the three discovery paths wired by the CLI: +// - SourceEnv — AWF_WORKFLOWS_PATH environment variable +// - SourceLocal — ./.awf/workflows/ (project-local) +// - SourceGlobal — $XDG_CONFIG_HOME/awf/workflows/ (user-wide) +type WorkflowSource string + +const ( + SourceEnv WorkflowSource = "env" + SourceLocal WorkflowSource = "local" + SourceGlobal WorkflowSource = "global" +) + +// WorkflowInfo carries the minimal metadata returned by ListWithSource. +// It lives in the ports package so that the application layer and the domain +// port interface share the same type without importing infrastructure packages. +type WorkflowInfo struct { + Name string + Source WorkflowSource + Path string +} + type WorkflowRepository interface { Load(ctx context.Context, name string) (*workflow.Workflow, error) List(ctx context.Context) ([]string, error) + // ListWithSource returns workflow names together with their discovery source. + // The ordering follows the same priority as List: earlier paths win for + // duplicates and are listed first. + ListWithSource(ctx context.Context) ([]WorkflowInfo, error) Exists(ctx context.Context, name string) (bool, error) } diff --git a/internal/domain/ports/repository_test.go b/internal/domain/ports/repository_test.go index 9f307f57..ccbb6d41 100644 --- a/internal/domain/ports/repository_test.go +++ b/internal/domain/ports/repository_test.go @@ -33,6 +33,15 @@ func (m *mockRepository) List(ctx context.Context) ([]string, error) { return names, nil } +func (m *mockRepository) ListWithSource(ctx context.Context) ([]ports.WorkflowInfo, error) { + names, _ := m.List(ctx) + infos := make([]ports.WorkflowInfo, 0, len(names)) + for _, name := range names { + infos = append(infos, ports.WorkflowInfo{Name: name, Source: ports.SourceLocal}) + } + return infos, nil +} + func (m *mockRepository) Exists(ctx context.Context, name string) (bool, error) { _, ok := m.workflows[name] return ok, nil diff --git a/internal/domain/workflow/entry.go b/internal/domain/workflow/entry.go index b18feb8f..c88f804f 100644 --- a/internal/domain/workflow/entry.go +++ b/internal/domain/workflow/entry.go @@ -1,11 +1,27 @@ package workflow -// WorkflowEntry represents a workflow from any source (local, global, pack) +// WorkflowEntry represents a workflow from any source (local, global, env, pack) // for listing purposes. It carries enough metadata for display without loading // the full workflow definition. +// +// Fields: +// - Name — display identifier; "workflow-name" for local/global/env, +// "packName/workflowName" for pack-sourced entries. +// - Source — provenance string matching the discovery origin: +// "local" (./.awf/workflows/), "global" ($XDG_CONFIG_HOME/awf/workflows/), +// "env" (AWF_WORKFLOWS_PATH), or "pack". +// - Scope — human-readable grouping label. Equals Source for non-pack +// entries ("local", "global", "env"). For pack-sourced entries it is the +// pack vendor name (e.g. "acme") rather than "pack". +// - Workflow — bare workflow name (no pack prefix); equals Name for +// local/global/env entries, the filename part for pack entries. +// - Version — optional semver string carried from the pack manifest. +// - Description — optional one-line summary surfaced in listings. type WorkflowEntry struct { Name string // "my-workflow" or "pack-name/workflow-name" Source string // "local", "global", "env", "pack" + Scope string // "local", "global", "env", or pack vendor name + Workflow string // local part: plain name for non-pack, wfName for pack Version string Description string } diff --git a/internal/domain/workflow/entry_test.go b/internal/domain/workflow/entry_test.go new file mode 100644 index 00000000..84a89e30 --- /dev/null +++ b/internal/domain/workflow/entry_test.go @@ -0,0 +1,154 @@ +package workflow + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestWorkflowEntry_LocalSource(t *testing.T) { + tests := []struct { + name string + entry WorkflowEntry + wantName string + wantSource string + wantScope string + wantWorkflow string + }{ + { + name: "local workflow", + entry: WorkflowEntry{ + Name: "deploy", + Source: "local", + Scope: "local", + Workflow: "deploy", + }, + wantName: "deploy", + wantSource: "local", + wantScope: "local", + wantWorkflow: "deploy", + }, + { + name: "global workflow scope mirrors source", + entry: WorkflowEntry{ + Name: "deploy", + Source: "global", + Scope: "global", + Workflow: "deploy", + }, + wantName: "deploy", + wantSource: "global", + wantScope: "global", + wantWorkflow: "deploy", + }, + { + name: "env workflow scope mirrors source", + entry: WorkflowEntry{ + Name: "audit", + Source: "env", + Scope: "env", + Workflow: "audit", + }, + wantName: "audit", + wantSource: "env", + wantScope: "env", + wantWorkflow: "audit", + }, + { + name: "local workflow with optional fields empty", + entry: WorkflowEntry{ + Name: "build", + Source: "local", + Scope: "local", + Workflow: "build", + }, + wantName: "build", + wantSource: "local", + wantScope: "local", + wantWorkflow: "build", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.wantName, tt.entry.Name) + assert.Equal(t, tt.wantSource, tt.entry.Source) + assert.Equal(t, tt.wantScope, tt.entry.Scope) + assert.Equal(t, tt.wantWorkflow, tt.entry.Workflow) + }) + } +} + +func TestWorkflowEntry_PackSource(t *testing.T) { + tests := []struct { + name string + entry WorkflowEntry + wantName string + wantSource string + wantScope string + wantWorkflow string + }{ + { + name: "pack workflow carries vendor scope", + entry: WorkflowEntry{ + Name: "speckit/specify", + Source: "pack", + Scope: "speckit", + Workflow: "specify", + }, + wantName: "speckit/specify", + wantSource: "pack", + wantScope: "speckit", + wantWorkflow: "specify", + }, + { + name: "pack workflow name differs from workflow bare name", + entry: WorkflowEntry{ + Name: "acme/deploy", + Source: "pack", + Scope: "acme", + Workflow: "deploy", + }, + wantName: "acme/deploy", + wantSource: "pack", + wantScope: "acme", + wantWorkflow: "deploy", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.wantName, tt.entry.Name) + assert.Equal(t, tt.wantSource, tt.entry.Source) + assert.Equal(t, tt.wantScope, tt.entry.Scope) + assert.Equal(t, tt.wantWorkflow, tt.entry.Workflow) + }) + } +} + +func TestWorkflowEntry_OptionalFields(t *testing.T) { + // Version and Description are optional and may be empty. + entry := WorkflowEntry{ + Name: "ci", + Source: "local", + Scope: "local", + Workflow: "ci", + } + + assert.Empty(t, entry.Version) + assert.Empty(t, entry.Description) +} + +func TestWorkflowEntry_WithVersionAndDescription(t *testing.T) { + entry := WorkflowEntry{ + Name: "speckit/specify", + Source: "pack", + Scope: "speckit", + Workflow: "specify", + Version: "1.2.0", + Description: "Spec-driven development workflow", + } + + assert.Equal(t, "1.2.0", entry.Version) + assert.Equal(t, "Spec-driven development workflow", entry.Description) +} diff --git a/internal/infrastructure/repository/composite_repository.go b/internal/infrastructure/repository/composite_repository.go index 2ab42f2e..5c19aa71 100644 --- a/internal/infrastructure/repository/composite_repository.go +++ b/internal/infrastructure/repository/composite_repository.go @@ -8,6 +8,7 @@ import ( "path/filepath" domerrors "github.com/awf-project/cli/internal/domain/errors" + "github.com/awf-project/cli/internal/domain/ports" "github.com/awf-project/cli/internal/domain/workflow" ) @@ -89,10 +90,12 @@ func (r *CompositeRepository) List(ctx context.Context) ([]string, error) { return names, nil } -// ListWithSource returns workflow info including source for each workflow -func (r *CompositeRepository) ListWithSource(ctx context.Context) ([]WorkflowInfo, error) { +// ListWithSource returns workflow info including source for each workflow. +// It implements ports.WorkflowRepository. The returned Source values map the +// internal Source iota to ports.WorkflowSource strings ("env", "local", "global"). +func (r *CompositeRepository) ListWithSource(ctx context.Context) ([]ports.WorkflowInfo, error) { seen := make(map[string]bool) - var infos []WorkflowInfo + var infos []ports.WorkflowInfo for _, sp := range r.paths { if !r.pathExists(sp.Path) { @@ -106,9 +109,9 @@ func (r *CompositeRepository) ListWithSource(ctx context.Context) ([]WorkflowInf for _, name := range repoNames { if !seen[name] { seen[name] = true - infos = append(infos, WorkflowInfo{ + infos = append(infos, ports.WorkflowInfo{ Name: name, - Source: sp.Source, + Source: ports.WorkflowSource(sp.Source.String()), Path: filepath.Join(sp.Path, name+".yaml"), }) } diff --git a/internal/infrastructure/repository/composite_repository_test.go b/internal/infrastructure/repository/composite_repository_test.go index 8eb89d90..d974d07f 100644 --- a/internal/infrastructure/repository/composite_repository_test.go +++ b/internal/infrastructure/repository/composite_repository_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" domerrors "github.com/awf-project/cli/internal/domain/errors" + "github.com/awf-project/cli/internal/domain/ports" ) func TestCompositeRepository_Load(t *testing.T) { @@ -189,15 +190,15 @@ states: require.NoError(t, err) // Build map for easy lookup - infoMap := make(map[string]WorkflowInfo) + infoMap := make(map[string]ports.WorkflowInfo) for _, info := range infos { infoMap[info.Name] = info } - assert.Equal(t, SourceLocal, infoMap["local-wf"].Source) - assert.Equal(t, SourceGlobal, infoMap["global-wf"].Source) + assert.Equal(t, ports.SourceLocal, infoMap["local-wf"].Source) + assert.Equal(t, ports.SourceGlobal, infoMap["global-wf"].Source) // shared-wf should show as local (higher priority) - assert.Equal(t, SourceLocal, infoMap["shared-wf"].Source) + assert.Equal(t, ports.SourceLocal, infoMap["shared-wf"].Source) }) } diff --git a/internal/infrastructure/repository/yaml_repository.go b/internal/infrastructure/repository/yaml_repository.go index 91787c25..447efbdd 100644 --- a/internal/infrastructure/repository/yaml_repository.go +++ b/internal/infrastructure/repository/yaml_repository.go @@ -11,17 +11,29 @@ import ( "gopkg.in/yaml.v3" domerrors "github.com/awf-project/cli/internal/domain/errors" + "github.com/awf-project/cli/internal/domain/ports" "github.com/awf-project/cli/internal/domain/workflow" "github.com/awf-project/cli/internal/infrastructure/expression" ) // YAMLRepository implements WorkflowRepository for YAML files. +// When used standalone (outside a CompositeRepository) it reports all workflows +// as SourceLocal — the caller may override this via WithSource if the repository +// is known to represent a different discovery origin. type YAMLRepository struct { - basePath string + basePath string + defaultSource Source } func NewYAMLRepository(basePath string) *YAMLRepository { - return &YAMLRepository{basePath: basePath} + return &YAMLRepository{basePath: basePath, defaultSource: SourceLocal} +} + +// WithSource returns a shallow copy of the repository configured to report the +// given Source in ListWithSource results. This is used by CompositeRepository +// to avoid creating separate YAMLRepository types per source. +func (r *YAMLRepository) WithSource(s Source) *YAMLRepository { + return &YAMLRepository{basePath: r.basePath, defaultSource: s} } // Load reads and parses a workflow from a YAML file. @@ -133,6 +145,25 @@ func (r *YAMLRepository) List(ctx context.Context) ([]string, error) { return names, nil } +// ListWithSource returns all workflow names together with their discovery source. +// Every entry carries the source configured on this repository (defaultSource), +// which is SourceLocal unless overridden via WithSource. +func (r *YAMLRepository) ListWithSource(ctx context.Context) ([]ports.WorkflowInfo, error) { + names, err := r.List(ctx) + if err != nil { + return nil, err + } + infos := make([]ports.WorkflowInfo, 0, len(names)) + for _, name := range names { + infos = append(infos, ports.WorkflowInfo{ + Name: name, + Source: ports.WorkflowSource(r.defaultSource.String()), + Path: filepath.Join(r.basePath, name+".yaml"), + }) + } + return infos, nil +} + // Exists checks if a workflow file exists. func (r *YAMLRepository) Exists(ctx context.Context, name string) (bool, error) { filePath := r.resolvePath(name) diff --git a/internal/infrastructure/workflowpkg/discoverer.go b/internal/infrastructure/workflowpkg/discoverer.go index 3430cdf1..164d6af7 100644 --- a/internal/infrastructure/workflowpkg/discoverer.go +++ b/internal/infrastructure/workflowpkg/discoverer.go @@ -43,6 +43,12 @@ func (a *PackDiscovererAdapter) DiscoverWorkflows(ctx context.Context) ([]workfl continue } for _, p := range packs { + // Defense-in-depth: skip pack names that fail the name regex even if + // the loader's Validate already rejects them. This prevents path + // traversal through a crafted pack name reaching filepath.Join. + if !nameRegex.MatchString(p.Name) { + continue + } if _, seen := packMap[p.Name]; !seen { packMap[p.Name] = filepath.Join(dir, p.Name) } @@ -67,9 +73,17 @@ func (a *PackDiscovererAdapter) DiscoverWorkflows(ctx context.Context) ([]workfl } for _, wfName := range manifest.Workflows { + // Defense-in-depth: skip workflow names that fail the name regex. + // Manifest.Validate already enforces this, but the second ParseManifest + // call (without Validate) in this path makes a defensive check necessary. + if !nameRegex.MatchString(wfName) { + continue + } entries = append(entries, workflow.WorkflowEntry{ Name: packName + "/" + wfName, Source: "pack", + Scope: packName, + Workflow: wfName, Version: manifest.Version, Description: loadWorkflowDescription(packDir, wfName), }) @@ -99,6 +113,13 @@ func (a *PackDiscovererAdapter) LoadWorkflow(ctx context.Context, packName, work // loadWorkflowDescription reads the description field from a workflow YAML file. // Returns an empty string if the file cannot be read or does not contain a description. func loadWorkflowDescription(packDir, workflowName string) string { + // Defense-in-depth: reject workflow names that would escape the workflows/ + // subdirectory. nameRegex already enforces this at the call site, but guard + // here too since loadWorkflowDescription is a package-internal helper that + // could be called directly. + if !nameRegex.MatchString(workflowName) { + return "" + } data, err := readFileLimited(filepath.Join(packDir, "workflows", workflowName+".yaml"), 1<<20) if err != nil { return "" diff --git a/internal/infrastructure/workflowpkg/discoverer_test.go b/internal/infrastructure/workflowpkg/discoverer_test.go index f3414fe2..d7d1f2c1 100644 --- a/internal/infrastructure/workflowpkg/discoverer_test.go +++ b/internal/infrastructure/workflowpkg/discoverer_test.go @@ -2,8 +2,10 @@ package workflowpkg_test import ( "context" + "fmt" "os" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -159,3 +161,147 @@ steps: require.Len(t, entries, 1) assert.Equal(t, "Greet the user", entries[0].Description) } + +// TestPackDiscovererAdapter_DiscoverWorkflows_SkipsPackWithInvalidName verifies that +// a pack whose manifest declares a name with path-traversal characters is silently +// skipped and does not appear in the results. No panic, no path escape. +func TestPackDiscovererAdapter_DiscoverWorkflows_SkipsPackWithInvalidName(t *testing.T) { + dir := t.TempDir() + // The directory name must be a valid OS path component; the evil name is in + // the manifest content, which is what nameRegex checks. + packDir := filepath.Join(dir, "evilpack") + require.NoError(t, os.MkdirAll(filepath.Join(packDir, "workflows"), 0o755)) + + // manifest.Name contains path traversal; Validate will reject it. + evilManifest := `name: "../../evil" +version: "1.0.0" +author: "test" +awf_version: ">=0.5.0" +workflows: + - hello +` + require.NoError(t, os.WriteFile(filepath.Join(packDir, "manifest.yaml"), []byte(evilManifest), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(packDir, "workflows", "hello.yaml"), []byte("name: hello\n"), 0o644)) + stateJSON := `{"name":"evilpack","enabled":true,"source_data":{"repository":"owner/evilpack","version":"1.0.0"}}` + require.NoError(t, os.WriteFile(filepath.Join(packDir, "state.json"), []byte(stateJSON), 0o644)) + + adapter := workflowpkg.NewPackDiscovererAdapter([]string{dir}) + entries, err := adapter.DiscoverWorkflows(context.Background()) + + require.NoError(t, err, "invalid pack name must not cause an error — it is silently skipped") + assert.Empty(t, entries, "pack with invalid name must not produce entries") +} + +// TestPackDiscovererAdapter_DiscoverWorkflows_SkipsPackWithInvalidWorkflowName verifies +// that a pack with a valid name but a manifest declaring invalid workflow names (including +// path traversal) is silently skipped and does not appear in the results. +func TestPackDiscovererAdapter_DiscoverWorkflows_SkipsPackWithInvalidWorkflowName(t *testing.T) { + dir := t.TempDir() + packDir := filepath.Join(dir, "safepack") + require.NoError(t, os.MkdirAll(filepath.Join(packDir, "workflows"), 0o755)) + + // Pack name is valid but workflow name contains path traversal. + // manifest.Validate now rejects invalid workflow names, so DiscoverPacks + // will skip this pack entirely. + evilManifest := `name: safepack +version: "1.0.0" +author: "test" +awf_version: ">=0.5.0" +workflows: + - "../../etc/passwd" +` + require.NoError(t, os.WriteFile(filepath.Join(packDir, "manifest.yaml"), []byte(evilManifest), 0o644)) + stateJSON := `{"name":"safepack","enabled":true,"source_data":{"repository":"owner/safepack","version":"1.0.0"}}` + require.NoError(t, os.WriteFile(filepath.Join(packDir, "state.json"), []byte(stateJSON), 0o644)) + + adapter := workflowpkg.NewPackDiscovererAdapter([]string{dir}) + entries, err := adapter.DiscoverWorkflows(context.Background()) + + require.NoError(t, err, "invalid workflow name must not cause an error — pack is silently skipped") + assert.Empty(t, entries, "pack with invalid workflow name must not produce entries") + + // Verify no entry name contains the traversal string. + for _, e := range entries { + assert.NotContains(t, e.Name, "..", "entry name must not contain path traversal") + assert.NotContains(t, e.Workflow, "..", "workflow field must not contain path traversal") + } +} + +// TestPackDiscovererAdapter_DiscoverWorkflows_PopulatesScopeAndWorkflowFields covers both single and multiple +// workflows per pack to ensure Scope=packName, Workflow=wfName, Name=packName/wfName, Source="pack". +func TestPackDiscovererAdapter_DiscoverWorkflows_PopulatesScopeAndWorkflowFields(t *testing.T) { + tests := []struct { + name string + packName string + workflows []string + wantScope string + wantWorkflow string + }{ + { + name: "single workflow in pack", + packName: "acme", + workflows: []string{"deploy"}, + wantScope: "acme", + wantWorkflow: "deploy", + }, + { + name: "multiple workflows in pack", + packName: "vendors", + workflows: []string{"build", "test", "deploy"}, + wantScope: "vendors", + wantWorkflow: "build", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + packDir := filepath.Join(dir, tt.packName) + require.NoError(t, os.MkdirAll(filepath.Join(packDir, "workflows"), 0o755)) + + var workflowsYAML strings.Builder + for _, wf := range tt.workflows { + fmt.Fprintf(&workflowsYAML, " - %s\n", wf) + } + + manifest := fmt.Sprintf(`name: %s +version: "1.0.0" +author: "test" +awf_version: ">=0.5.0" +workflows: +%s`, tt.packName, workflowsYAML.String()) + require.NoError(t, os.WriteFile(filepath.Join(packDir, "manifest.yaml"), []byte(manifest), 0o644)) + + for _, wf := range tt.workflows { + wfYAML := fmt.Sprintf(`name: %s +initial: start +steps: + start: + type: terminal + status: success + message: ok +`, wf) + require.NoError(t, os.WriteFile( + filepath.Join(packDir, "workflows", wf+".yaml"), + []byte(wfYAML), + 0o644, + )) + } + + stateJSON := fmt.Sprintf(`{"name":%q,"enabled":true,"source_data":{"repository":"owner/%s","version":"1.0.0"}}`, tt.packName, tt.packName) + require.NoError(t, os.WriteFile(filepath.Join(packDir, "state.json"), []byte(stateJSON), 0o644)) + + adapter := workflowpkg.NewPackDiscovererAdapter([]string{dir}) + entries, err := adapter.DiscoverWorkflows(context.Background()) + + require.NoError(t, err) + require.NotEmpty(t, entries, "should discover at least one workflow") + + entry := entries[0] + assert.Equal(t, tt.packName+"/"+tt.wantWorkflow, entry.Name, "Name should be pack/workflow") + assert.Equal(t, tt.wantScope, entry.Scope, "Scope should be pack name") + assert.Equal(t, tt.wantWorkflow, entry.Workflow, "Workflow should be workflow name") + assert.Equal(t, "pack", entry.Source, "Source should be pack") + }) + } +} diff --git a/internal/infrastructure/workflowpkg/manifest.go b/internal/infrastructure/workflowpkg/manifest.go index ec049360..54fa0136 100644 --- a/internal/infrastructure/workflowpkg/manifest.go +++ b/internal/infrastructure/workflowpkg/manifest.go @@ -6,6 +6,7 @@ import ( "path/filepath" "regexp" + domerrors "github.com/awf-project/cli/internal/domain/errors" "github.com/awf-project/cli/pkg/registry" "gopkg.in/yaml.v3" ) @@ -48,6 +49,15 @@ func (m *Manifest) Validate(packDir string) error { return fmt.Errorf("manifest: invalid pack name %q (must match ^[a-z][a-z0-9-]*$)", m.Name) } + if m.Name == "local" || m.Name == "global" || m.Name == "env" { + return domerrors.NewUserError( + domerrors.ErrorCodeUserInputValidationFailed, + fmt.Sprintf("pack name %q is reserved as a scope sentinel; rename the pack", m.Name), + map[string]any{"pack_name": m.Name, "reserved_tokens": []string{"local", "global", "env"}}, + nil, + ) + } + if _, err := registry.ParseVersion(m.Version); err != nil { return fmt.Errorf("manifest: invalid version %q: %w", m.Version, err) } @@ -66,6 +76,9 @@ func (m *Manifest) Validate(packDir string) error { } for _, workflow := range m.Workflows { + if !nameRegex.MatchString(workflow) { + return fmt.Errorf("manifest: invalid workflow name %q (must match ^[a-z][a-z0-9-]*$)", workflow) + } workflowFile := filepath.Join(workflowsDir, workflow+".yaml") if _, err := os.Stat(workflowFile); err != nil { return fmt.Errorf("manifest: workflow file %q not found", workflow+".yaml") diff --git a/internal/infrastructure/workflowpkg/manifest_test.go b/internal/infrastructure/workflowpkg/manifest_test.go index e9efd20d..cf4ea438 100644 --- a/internal/infrastructure/workflowpkg/manifest_test.go +++ b/internal/infrastructure/workflowpkg/manifest_test.go @@ -1,10 +1,12 @@ package workflowpkg_test import ( + "errors" "os" "path/filepath" "testing" + domerrors "github.com/awf-project/cli/internal/domain/errors" "github.com/awf-project/cli/internal/infrastructure/workflowpkg" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -279,3 +281,184 @@ func TestValidate_EmptyWorkflowsList(t *testing.T) { assert.Error(t, err) } + +// TestValidate_InvalidWorkflowName tests that workflow names with path traversal +// or invalid characters are rejected to prevent path traversal attacks. +func TestValidate_InvalidWorkflowName(t *testing.T) { + tests := []struct { + name string + workflowName string + }{ + {name: "path traversal", workflowName: "../../etc/passwd"}, + {name: "absolute path", workflowName: "/etc/passwd"}, + {name: "starts with digit", workflowName: "1workflow"}, + {name: "contains uppercase", workflowName: "MyWorkflow"}, + {name: "contains underscore", workflowName: "my_workflow"}, + {name: "contains slash", workflowName: "pack/workflow"}, + {name: "empty name", workflowName: ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + packDir := t.TempDir() + workflowsDir := filepath.Join(packDir, "workflows") + require.NoError(t, os.Mkdir(workflowsDir, 0o755)) + + manifest := &workflowpkg.Manifest{ + Name: "valid", + Version: "1.0.0", + Description: "Test", + Author: "test", + AWFVersion: ">=0.1.0", + Workflows: []string{tt.workflowName}, + } + + err := manifest.Validate(packDir) + + assert.Error(t, err, "expected validation to fail for workflow name %q", tt.workflowName) + assert.Contains(t, err.Error(), "invalid workflow name", "error should mention invalid workflow name") + }) + } +} + +// TestValidate_ValidWorkflowNames tests that well-formed kebab-case names are accepted. +func TestValidate_ValidWorkflowNames(t *testing.T) { + tests := []struct { + name string + workflowName string + }{ + {name: "simple lowercase", workflowName: "specify"}, + {name: "kebab-case", workflowName: "run-tests"}, + {name: "with digits", workflowName: "deploy-v2"}, + {name: "single letter", workflowName: "a"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + packDir := t.TempDir() + workflowsDir := filepath.Join(packDir, "workflows") + require.NoError(t, os.Mkdir(workflowsDir, 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(workflowsDir, tt.workflowName+".yaml"), + []byte("test"), + 0o644, + )) + + manifest := &workflowpkg.Manifest{ + Name: "valid", + Version: "1.0.0", + Description: "Test", + Author: "test", + AWFVersion: ">=0.1.0", + Workflows: []string{tt.workflowName}, + } + + err := manifest.Validate(packDir) + + assert.NoError(t, err) + }) + } +} + +func TestValidate_ReservedScope(t *testing.T) { + tests := []struct { + name string + packName string + wantErr bool + wantCode domerrors.ErrorCode + wantPackName string + wantFormatErr bool + }{ + { + name: "local is reserved", + packName: "local", + wantErr: true, + wantCode: domerrors.ErrorCodeUserInputValidationFailed, + wantPackName: "local", + }, + { + name: "global is reserved", + packName: "global", + wantErr: true, + wantCode: domerrors.ErrorCodeUserInputValidationFailed, + wantPackName: "global", + }, + { + name: "env is reserved", + packName: "env", + wantErr: true, + wantCode: domerrors.ErrorCodeUserInputValidationFailed, + wantPackName: "env", + }, + { + name: "localpack is not reserved", + packName: "localpack", + wantErr: false, + }, + { + name: "globalpack is not reserved", + packName: "globalpack", + wantErr: false, + }, + { + name: "envpack is not reserved", + packName: "envpack", + wantErr: false, + }, + { + name: "run is not reserved", + packName: "run", + wantErr: false, + }, + { + name: "validate is not reserved", + packName: "validate", + wantErr: false, + }, + { + name: "invalid format fails with format error not reserved error", + packName: "!!!", + wantErr: true, + wantFormatErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + packDir := t.TempDir() + workflowsDir := filepath.Join(packDir, "workflows") + require.NoError(t, os.Mkdir(workflowsDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(workflowsDir, "test.yaml"), []byte("test"), 0o644)) + + manifest := &workflowpkg.Manifest{ + Name: tt.packName, + Version: "1.0.0", + Description: "Test", + Author: "test", + AWFVersion: ">=0.1.0", + Workflows: []string{"test"}, + } + + err := manifest.Validate(packDir) + + if !tt.wantErr { + assert.NoError(t, err) + return + } + + require.Error(t, err) + + if tt.wantFormatErr { + var structErr *domerrors.StructuredError + assert.False(t, errors.As(err, &structErr), "format error must not be a StructuredError") + return + } + + var structErr *domerrors.StructuredError + require.True(t, errors.As(err, &structErr), "expected *StructuredError") + assert.Equal(t, tt.wantCode, structErr.Code) + assert.Equal(t, tt.wantPackName, structErr.Details["pack_name"]) + assert.Equal(t, []string{"local", "global", "env"}, structErr.Details["reserved_tokens"]) + }) + } +} diff --git a/internal/interfaces/api/bridge.go b/internal/interfaces/api/bridge.go index 96971199..26690c9e 100644 --- a/internal/interfaces/api/bridge.go +++ b/internal/interfaces/api/bridge.go @@ -144,7 +144,11 @@ func (b *Bridge) ListExecutions() []*ActiveExecution { // TrackResumedExecution wraps a synchronously-resumed ExecutionContext in an // ActiveExecution, assigns it a new UUID, stores it in activeExecutions, and // returns the assigned ID. Because resume is synchronous the execution is -// already complete; no background context or cancel is needed. +// already complete when this returns; the entry is kept intentionally so that +// subsequent GET /api/executions/{id} (and DELETE / SSE endpoints) can serve +// the terminal state to clients querying the just-resumed execution. Without +// this persistence the /resume handler would return an ID that immediately +// 404s on read. Eviction/TTL of completed entries is a separate concern. func (b *Bridge) TrackResumedExecution(execCtx *workflow.ExecutionContext) string { id := uuid.NewString() closed := make(chan error) diff --git a/internal/interfaces/api/bridge_test.go b/internal/interfaces/api/bridge_test.go index b9259b66..e1bc8b5a 100644 --- a/internal/interfaces/api/bridge_test.go +++ b/internal/interfaces/api/bridge_test.go @@ -3,6 +3,7 @@ package api import ( "context" "errors" + "strings" "testing" "time" @@ -15,11 +16,13 @@ import ( // --- mock implementations of Bridge interfaces --- type mockWorkflowLister struct { - entries []workflow.WorkflowEntry - wfs map[string]*workflow.Workflow - listErr error - getErr error - validErr error + entries []workflow.WorkflowEntry + wfs map[string]*workflow.Workflow + listErr error + getErr error + validErr error + lastGetName string + lastValidateName string } func newMockWorkflowLister(names ...string) *mockWorkflowLister { @@ -28,7 +31,17 @@ func newMockWorkflowLister(names ...string) *mockWorkflowLister { wfs: make(map[string]*workflow.Workflow, len(names)), } for _, name := range names { - m.entries = append(m.entries, workflow.WorkflowEntry{Name: name, Source: "local"}) + scope, wfName, _ := strings.Cut(name, "/") + if !strings.Contains(name, "/") { + scope = "local" + wfName = name + } + m.entries = append(m.entries, workflow.WorkflowEntry{ + Name: name, + Source: "local", + Scope: scope, + Workflow: wfName, + }) m.wfs[name] = &workflow.Workflow{ Name: name, Steps: map[string]*workflow.Step{"step-1": {Name: "step-1"}}, @@ -45,6 +58,7 @@ func (m *mockWorkflowLister) ListAllWorkflows(_ context.Context) ([]workflow.Wor } func (m *mockWorkflowLister) GetWorkflow(_ context.Context, name string) (*workflow.Workflow, error) { + m.lastGetName = name if m.getErr != nil { return nil, m.getErr } @@ -55,7 +69,8 @@ func (m *mockWorkflowLister) GetWorkflow(_ context.Context, name string) (*workf return wf, nil } -func (m *mockWorkflowLister) ValidateWorkflow(_ context.Context, _ string) error { +func (m *mockWorkflowLister) ValidateWorkflow(_ context.Context, name string) error { + m.lastValidateName = name return m.validErr } @@ -234,6 +249,25 @@ func TestBridge_GetExecution_LiveSnapshot(t *testing.T) { assert.Equal(t, "wf-1", exec.WorkflowName) } +func TestBridge_TrackResumedExecution_PersistsEntryForSubsequentQueries(t *testing.T) { + // Resumed executions are intentionally persisted so the /resume handler can + // return an ID that subsequent GET /api/executions/{id} (and the SSE/DELETE + // endpoints) can serve. Eviction/TTL of completed entries is out of scope + // here; this test guards against accidental immediate-cleanup regressions. + bridge := NewBridge(newMockWorkflowLister(), newMockWorkflowRunner(), newMockHistoryProvider()) + execCtx := workflow.NewExecutionContext("resumed-001", "my-workflow") + + id := bridge.TrackResumedExecution(execCtx) + require.NotEmpty(t, id, "must return a non-empty execution ID") + + stored, ok := bridge.GetExecution(id) + require.True(t, ok, "entry must be queryable immediately after TrackResumedExecution") + require.NotNil(t, stored) + assert.Equal(t, id, stored.ExecutionID) + assert.Equal(t, "my-workflow", stored.WorkflowName) + assert.Same(t, execCtx, stored.ExecutionContext) +} + func TestBridge_ListExecutions_ReturnsActiveAndCompleted(t *testing.T) { // Use blocking channels to prevent cleanup goroutine from removing entries blockA := make(chan error) diff --git a/internal/interfaces/api/doc.go b/internal/interfaces/api/doc.go index 0fdd9aaf..8f5f99ca 100644 --- a/internal/interfaces/api/doc.go +++ b/internal/interfaces/api/doc.go @@ -12,9 +12,9 @@ // The api package provides a REST API surface for AWF with the following // capabilities: // -// - Workflow discovery and validation (GET /api/workflows, GET /api/workflows/{name}, -// POST /api/workflows/{name}/validate) -// - Asynchronous workflow execution (POST /api/workflows/{name}/run) +// - Workflow discovery and validation (GET /api/workflows, GET /api/workflows/{scope}/{name}, +// POST /api/workflows/{scope}/{name}/validate) +// - Asynchronous workflow execution (POST /api/workflows/{scope}/{name}/run) // - Execution lifecycle management (GET/DELETE /api/executions/{id}, // POST /api/executions/{id}/resume) // - Real-time event streaming via Server-Sent Events (GET /api/executions/{id}/events) @@ -141,7 +141,7 @@ // // ## Async executions // -// RunWorkflow (POST /api/workflows/{name}/run) starts workflow execution in a +// RunWorkflow (POST /api/workflows/{scope}/{name}/run) starts workflow execution in a // background goroutine managed by Bridge.StartExecution. The client receives // 202 Accepted with an execution ID immediately. Subsequent calls to // GET /api/executions/{id} or the SSE stream observe the running state. diff --git a/internal/interfaces/api/handlers_executions.go b/internal/interfaces/api/handlers_executions.go index cdbd582f..34293e6d 100644 --- a/internal/interfaces/api/handlers_executions.go +++ b/internal/interfaces/api/handlers_executions.go @@ -18,16 +18,17 @@ func NewExecutionHandlers(b *Bridge) *ExecutionHandlers { } func (h *ExecutionHandlers) Run(ctx context.Context, in *RunWorkflowInput) (*RunWorkflowOutput, error) { - wf, err := h.b.workflows.GetWorkflow(ctx, in.Name) + id := recomposeIdentifier(in.Scope, in.Name) + wf, err := h.b.workflows.GetWorkflow(ctx, id) if err != nil { - return nil, huma.Error404NotFound(fmt.Sprintf("workflow not found: %s", in.Name)) + return nil, huma.Error404NotFound(fmt.Sprintf("workflow not found: %s", id)) } - id, _, err := h.b.StartExecution(ctx, wf, in.Body.Inputs) + execID, _, err := h.b.StartExecution(ctx, wf, in.Body.Inputs) if err != nil { return nil, huma.Error422UnprocessableEntity(fmt.Sprintf("failed to start execution: %s", err)) } out := &RunWorkflowOutput{} - out.Body.Body = runWorkflowBody{ExecutionID: id, Status: "accepted"} + out.Body.Body = runWorkflowBody{ExecutionID: execID, Status: "accepted"} return out, nil } @@ -89,7 +90,7 @@ func activeExecutionToBody(ae *ActiveExecution) executionBody { func RegisterExecutionRoutes(api huma.API, h *ExecutionHandlers) { huma.Register(api, huma.Operation{ Method: "POST", - Path: "/api/workflows/{name}/run", + Path: "/api/workflows/{scope}/{name}/run", OperationID: "run-workflow", Tags: []string{"Executions"}, DefaultStatus: 202, diff --git a/internal/interfaces/api/handlers_executions_test.go b/internal/interfaces/api/handlers_executions_test.go index c7e5e842..06880f53 100644 --- a/internal/interfaces/api/handlers_executions_test.go +++ b/internal/interfaces/api/handlers_executions_test.go @@ -38,19 +38,29 @@ func (m *mockWorkflowResumer) Resume( return m.execCtx, nil } -// --- Tests --- - -func TestExecutionHandler_Run_Returns202WithExecutionID_WithinDeadline(t *testing.T) { - // Blocking channel prevents cleanup goroutine from removing the entry before assertions. +// newBlockingExecutionHandlerAPI wires a full execution-handler test stack with a +// blocking runner. The runner's Done channel stays open until test cleanup, which +// prevents Bridge's cleanup goroutine in StartExecution from removing tracked +// executions before assertions run. Returns the api (for HTTP calls), the bridge +// (for GetExecution/ListExecutions assertions), and the lister (so callers can +// mutate getErr / entries fields before issuing requests). +func newBlockingExecutionHandlerAPI(t *testing.T, workflowNames ...string) (humatest.TestAPI, *Bridge, *mockWorkflowLister) { + t.Helper() block := make(chan error) t.Cleanup(func() { close(block) }) - - lister := newMockWorkflowLister("deploy-prod") + lister := newMockWorkflowLister(workflowNames...) runner := newMockWorkflowRunnerWithDone(block) bridge := NewBridge(lister, runner, newMockHistoryProvider()) handler := NewExecutionHandlers(bridge) _, api := humatest.New(t) RegisterExecutionRoutes(api, handler) + return api, bridge, lister +} + +// --- Tests --- + +func TestExecutionHandler_Run_Returns202WithExecutionID_WithinDeadline(t *testing.T) { + api, bridge, _ := newBlockingExecutionHandlerAPI(t, "deploy-prod") // Verify the response is built and returned BEFORE the async work completes. // FR-006 deadline: 100ms from request receipt. @@ -66,15 +76,13 @@ func TestExecutionHandler_Run_Returns202WithExecutionID_WithinDeadline(t *testin Inputs: map[string]any{"env": "prod"}, } - resp := api.Post("/api/workflows/deploy-prod/run", input) + resp := api.Post("/api/workflows/local/deploy-prod/run", input) elapsed := time.Since(startTime) timeout.Stop() - // Assert HTTP 202 Accepted (async). require.Equal(t, 202, resp.Code, "Run must return 202 Accepted for async execution") - // Assert the execution ID is returned and non-empty. var result struct { Body struct { ExecutionID string `json:"execution_id"` @@ -88,7 +96,6 @@ func TestExecutionHandler_Run_Returns202WithExecutionID_WithinDeadline(t *testin assert.Equal(t, "accepted", result.Body.Status, "status must be 'accepted'") assert.Less(t, elapsed, 100*time.Millisecond, "handler must return within FR-006 deadline") - // Verify the execution is tracked in the Bridge. stored, ok := bridge.GetExecution(result.Body.ExecutionID) assert.True(t, ok, "execution must be stored in Bridge") require.NotNil(t, stored) @@ -110,7 +117,7 @@ func TestExecutionHandler_Run_UnknownWorkflow_Returns404(t *testing.T) { Inputs: map[string]any{}, } - resp := api.Post("/api/workflows/nonexistent/run", input) + resp := api.Post("/api/workflows/local/nonexistent/run", input) assert.Equal(t, 404, resp.Code, "Run with unknown workflow must return 404 Not Found") } @@ -162,15 +169,7 @@ func TestExecutionHandler_List_HappyPath(t *testing.T) { } func TestExecutionHandler_Get_HappyPath(t *testing.T) { - block := make(chan error) - t.Cleanup(func() { close(block) }) - - lister := newMockWorkflowLister("test-workflow") - runner := newMockWorkflowRunnerWithDone(block) - bridge := NewBridge(lister, runner, newMockHistoryProvider()) - handler := NewExecutionHandlers(bridge) - _, api := humatest.New(t) - RegisterExecutionRoutes(api, handler) + api, bridge, _ := newBlockingExecutionHandlerAPI(t, "test-workflow") // Start an execution to get a valid ID. wf := &workflow.Workflow{Name: "test-workflow", Steps: map[string]*workflow.Step{"s1": {Name: "s1"}}} @@ -207,16 +206,7 @@ func TestExecutionHandler_Get_NotFound_Returns404(t *testing.T) { } func TestExecutionHandler_Cancel_PropagatesContextCancellation(t *testing.T) { - // Blocking channel prevents cleanup goroutine from removing the entry. - block := make(chan error) - t.Cleanup(func() { close(block) }) - - lister := newMockWorkflowLister("test-workflow") - runner := newMockWorkflowRunnerWithDone(block) - bridge := NewBridge(lister, runner, newMockHistoryProvider()) - handler := NewExecutionHandlers(bridge) - _, api := humatest.New(t) - RegisterExecutionRoutes(api, handler) + api, bridge, _ := newBlockingExecutionHandlerAPI(t, "test-workflow") // Start an execution. wf := &workflow.Workflow{Name: "test-workflow", Steps: map[string]*workflow.Step{"s1": {Name: "s1"}}} @@ -241,16 +231,7 @@ func TestExecutionHandler_Cancel_PropagatesContextCancellation(t *testing.T) { } func TestExecutionHandler_Cancel_Idempotent_TwoDELETEsBothReturn204(t *testing.T) { - // Blocking channel prevents cleanup goroutine from removing the entry during test. - block := make(chan error) - t.Cleanup(func() { close(block) }) - - lister := newMockWorkflowLister("test-workflow") - runner := newMockWorkflowRunnerWithDone(block) - bridge := NewBridge(lister, runner, newMockHistoryProvider()) - handler := NewExecutionHandlers(bridge) - _, api := humatest.New(t) - RegisterExecutionRoutes(api, handler) + api, bridge, _ := newBlockingExecutionHandlerAPI(t, "test-workflow") // Start an execution. wf := &workflow.Workflow{Name: "test-workflow", Steps: map[string]*workflow.Step{"s1": {Name: "s1"}}} @@ -288,23 +269,89 @@ func TestExecutionHandler_Cancel_UnknownID_Returns204(t *testing.T) { assert.Equal(t, 204, resp.Code, "Cancel with unknown ID must return 204 (idempotent)") } -func TestExecutionHandler_Resume_FailedExecution_RestartsFromFailedStep(t *testing.T) { - // Setup: execution stored in Bridge, resumer mocked. - block := make(chan error) - t.Cleanup(func() { close(block) }) +func TestExecutionHandler_Run_PackScope_Returns202_CanonicalNamePassed(t *testing.T) { + api, _, lister := newBlockingExecutionHandlerAPI(t, "speckit/specify") - lister := newMockWorkflowLister("test-workflow") - runner := newMockWorkflowRunnerWithDone(block) - bridge := NewBridge(lister, runner, newMockHistoryProvider()) + input := struct { + Inputs map[string]any `json:"inputs"` + }{ + Inputs: map[string]any{}, + } - // Wire the resumer. - resumer := newMockWorkflowResumer() - bridge.SetResumer(resumer) + resp := api.Post("/api/workflows/speckit/specify/run", input) + require.Equal(t, 202, resp.Code) + + var result struct { + Body struct { + ExecutionID string `json:"execution_id"` + Status string `json:"status"` + } `json:"body"` + } + err := json.NewDecoder(resp.Body).Decode(&result) + require.NoError(t, err) + assert.NotEmpty(t, result.Body.ExecutionID) + assert.Equal(t, "accepted", result.Body.Status) + assert.Equal(t, "speckit/specify", lister.lastGetName, "mock must receive canonical name for pack scope") +} + +func TestExecutionHandler_Run_LocalScope_Returns202_NameOnlyPassed(t *testing.T) { + api, _, lister := newBlockingExecutionHandlerAPI(t, "deploy-prod") + + input := struct { + Inputs map[string]any `json:"inputs"` + }{ + Inputs: map[string]any{}, + } + + resp := api.Post("/api/workflows/local/deploy-prod/run", input) + require.Equal(t, 202, resp.Code) + + var result struct { + Body struct { + ExecutionID string `json:"execution_id"` + Status string `json:"status"` + } `json:"body"` + } + err := json.NewDecoder(resp.Body).Decode(&result) + require.NoError(t, err) + + assert.NotEmpty(t, result.Body.ExecutionID) + assert.Equal(t, "accepted", result.Body.Status) + assert.Equal(t, "deploy-prod", lister.lastGetName, "mock must receive name only for local scope") +} + +func TestExecutionHandler_Run_UnknownScope_Returns404(t *testing.T) { + // This test asserts the 404 response when the workflow lookup fails, so no + // execution is ever started or tracked. We intentionally bypass + // newBlockingExecutionHandlerAPI (which wires a blocking runner to keep + // executions observable) and use the plain non-blocking mock runner. + lister := newMockWorkflowLister() + runner := newMockWorkflowRunner() + bridge := NewBridge(lister, runner, newMockHistoryProvider()) handler := NewExecutionHandlers(bridge) _, api := humatest.New(t) RegisterExecutionRoutes(api, handler) + input := struct { + Inputs map[string]any `json:"inputs"` + }{ + Inputs: map[string]any{}, + } + + resp := api.Post("/api/workflows/unknown/foo/run", input) + + assert.Equal(t, 404, resp.Code, "Run with unknown scope must return 404 Not Found") +} + +func TestExecutionHandler_Resume_FailedExecution_RestartsFromFailedStep(t *testing.T) { + // Setup: execution stored in Bridge, resumer mocked. + api, bridge, _ := newBlockingExecutionHandlerAPI(t, "test-workflow") + + // Wire the resumer. + resumer := newMockWorkflowResumer() + bridge.SetResumer(resumer) + // Start an execution (represents the failed run). wf := &workflow.Workflow{Name: "test-workflow", Steps: map[string]*workflow.Step{"s1": {Name: "s1"}}} failedID, _, err := bridge.StartExecution(context.Background(), wf, nil) diff --git a/internal/interfaces/api/handlers_workflows.go b/internal/interfaces/api/handlers_workflows.go index 26d3fb7a..a69dd9d4 100644 --- a/internal/interfaces/api/handlers_workflows.go +++ b/internal/interfaces/api/handlers_workflows.go @@ -7,7 +7,9 @@ import ( "github.com/danielgtaylor/huma/v2" ) -// WorkflowHandlers exposes workflow read operations via HTTP. +// WorkflowHandlers exposes workflow read operations (list, get, validate) via +// HTTP. It is bound to a Bridge which holds the WorkflowLister adapter to the +// application service layer. type WorkflowHandlers struct { b *Bridge } @@ -26,6 +28,8 @@ func (h *WorkflowHandlers) List(ctx context.Context, _ *struct{}) (*ListWorkflow for _, e := range entries { summaries = append(summaries, WorkflowSummary{ Name: e.Name, + Scope: e.Scope, + Workflow: e.Workflow, Version: e.Version, Description: e.Description, }) @@ -36,9 +40,10 @@ func (h *WorkflowHandlers) List(ctx context.Context, _ *struct{}) (*ListWorkflow } func (h *WorkflowHandlers) Get(ctx context.Context, in *GetWorkflowInput) (*GetWorkflowOutput, error) { - wf, err := h.b.workflows.GetWorkflow(ctx, in.Name) + id := recomposeIdentifier(in.Scope, in.Name) + wf, err := h.b.workflows.GetWorkflow(ctx, id) if err != nil { - return nil, huma.Error404NotFound(fmt.Sprintf("workflow not found: %s", in.Name)) + return nil, huma.Error404NotFound(fmt.Sprintf("workflow not found: %s", id)) } out := &GetWorkflowOutput{} out.Body.Body = wf @@ -46,8 +51,9 @@ func (h *WorkflowHandlers) Get(ctx context.Context, in *GetWorkflowInput) (*GetW } func (h *WorkflowHandlers) Validate(ctx context.Context, in *ValidateWorkflowInput) (*ValidateWorkflowOutput, error) { + id := recomposeIdentifier(in.Scope, in.Name) out := &ValidateWorkflowOutput{} - err := h.b.workflows.ValidateWorkflow(ctx, in.Name) + err := h.b.workflows.ValidateWorkflow(ctx, id) if err != nil { out.Body.Body = validateWorkflowBody{Errors: []string{err.Error()}} } @@ -65,14 +71,14 @@ func RegisterWorkflowRoutes(api huma.API, h *WorkflowHandlers) { huma.Register(api, huma.Operation{ Method: "GET", - Path: "/api/workflows/{name}", + Path: "/api/workflows/{scope}/{name}", OperationID: "get-workflow", Tags: []string{"Workflows"}, }, h.Get) huma.Register(api, huma.Operation{ Method: "POST", - Path: "/api/workflows/{name}/validate", + Path: "/api/workflows/{scope}/{name}/validate", OperationID: "validate-workflow", Tags: []string{"Workflows"}, }, h.Validate) diff --git a/internal/interfaces/api/handlers_workflows_test.go b/internal/interfaces/api/handlers_workflows_test.go index 0b900752..be8a9536 100644 --- a/internal/interfaces/api/handlers_workflows_test.go +++ b/internal/interfaces/api/handlers_workflows_test.go @@ -12,6 +12,19 @@ import ( "github.com/awf-project/cli/internal/domain/workflow" ) +// newWorkflowHandlerAPI wires a Bridge + WorkflowHandlers + humatest API +// around the given mock lister and returns the API for assertions. Bridge is +// constructed with nil runner/history because workflow-handler tests never +// exercise execution or history paths. +func newWorkflowHandlerAPI(t *testing.T, lister WorkflowLister) humatest.TestAPI { + t.Helper() + bridge := NewBridge(lister, nil, nil) + handler := NewWorkflowHandlers(bridge) + _, api := humatest.New(t) + RegisterWorkflowRoutes(api, handler) + return api +} + func TestWorkflowHandler_List_HappyPath(t *testing.T) { mock := newMockWorkflowLister("deploy-prod", "test-service") mock.entries[0].Version = "1.0.0" @@ -19,10 +32,7 @@ func TestWorkflowHandler_List_HappyPath(t *testing.T) { mock.entries[1].Version = "2.0.0" mock.entries[1].Description = "Run tests" - bridge := NewBridge(mock, nil, nil) - handler := NewWorkflowHandlers(bridge) - _, api := humatest.New(t) - RegisterWorkflowRoutes(api, handler) + api := newWorkflowHandlerAPI(t, mock) resp := api.Get("/api/workflows") require.Equal(t, 200, resp.Code) @@ -39,18 +49,17 @@ func TestWorkflowHandler_List_HappyPath(t *testing.T) { assert.Equal(t, "deploy-prod", result.Body.Workflows[0].Name) assert.Equal(t, "1.0.0", result.Body.Workflows[0].Version) assert.Equal(t, "Deploy to production", result.Body.Workflows[0].Description) + assert.Equal(t, "local", result.Body.Workflows[0].Scope) + assert.Equal(t, "deploy-prod", result.Body.Workflows[0].Workflow) } func TestWorkflowHandler_Get_NotFound_Returns404(t *testing.T) { mock := newMockWorkflowLister() mock.getErr = errors.New("workflow not found") - bridge := NewBridge(mock, nil, nil) - handler := NewWorkflowHandlers(bridge) - _, api := humatest.New(t) - RegisterWorkflowRoutes(api, handler) + api := newWorkflowHandlerAPI(t, mock) - resp := api.Get("/api/workflows/nonexistent") + resp := api.Get("/api/workflows/local/nonexistent") assert.Equal(t, 404, resp.Code) } @@ -58,10 +67,7 @@ func TestWorkflowHandler_Validate_InvalidWorkflow_ReturnsErrors(t *testing.T) { mock := newMockWorkflowLister("bad-workflow") mock.validErr = errors.New("invalid step reference") - bridge := NewBridge(mock, nil, nil) - handler := NewWorkflowHandlers(bridge) - _, api := humatest.New(t) - RegisterWorkflowRoutes(api, handler) + api := newWorkflowHandlerAPI(t, mock) validateInput := struct { Body struct { @@ -69,7 +75,7 @@ func TestWorkflowHandler_Validate_InvalidWorkflow_ReturnsErrors(t *testing.T) { } `json:"body"` }{} - resp := api.Post("/api/workflows/bad-workflow/validate", validateInput) + resp := api.Post("/api/workflows/local/bad-workflow/validate", validateInput) require.Equal(t, 200, resp.Code) var result struct { @@ -86,10 +92,7 @@ func TestWorkflowHandler_Validate_InvalidWorkflow_ReturnsErrors(t *testing.T) { func TestWorkflowHandler_List_EmptyList(t *testing.T) { mock := newMockWorkflowLister() - bridge := NewBridge(mock, nil, nil) - handler := NewWorkflowHandlers(bridge) - _, api := humatest.New(t) - RegisterWorkflowRoutes(api, handler) + api := newWorkflowHandlerAPI(t, mock) resp := api.Get("/api/workflows") require.Equal(t, 200, resp.Code) @@ -108,12 +111,9 @@ func TestWorkflowHandler_List_EmptyList(t *testing.T) { func TestWorkflowHandler_Get_FoundWorkflow_ReturnsWorkflow(t *testing.T) { mock := newMockWorkflowLister("test-workflow") - bridge := NewBridge(mock, nil, nil) - handler := NewWorkflowHandlers(bridge) - _, api := humatest.New(t) - RegisterWorkflowRoutes(api, handler) + api := newWorkflowHandlerAPI(t, mock) - resp := api.Get("/api/workflows/test-workflow") + resp := api.Get("/api/workflows/local/test-workflow") require.Equal(t, 200, resp.Code) var result struct { @@ -130,10 +130,7 @@ func TestWorkflowHandler_Validate_ValidWorkflow_ReturnsEmptyErrors(t *testing.T) mock := newMockWorkflowLister("valid-workflow") // validErr defaults to nil, which means validation passed - bridge := NewBridge(mock, nil, nil) - handler := NewWorkflowHandlers(bridge) - _, api := humatest.New(t) - RegisterWorkflowRoutes(api, handler) + api := newWorkflowHandlerAPI(t, mock) validateInput := struct { Body struct { @@ -141,7 +138,7 @@ func TestWorkflowHandler_Validate_ValidWorkflow_ReturnsEmptyErrors(t *testing.T) } `json:"body"` }{} - resp := api.Post("/api/workflows/valid-workflow/validate", validateInput) + resp := api.Post("/api/workflows/local/valid-workflow/validate", validateInput) require.Equal(t, 200, resp.Code) var result struct { @@ -154,3 +151,83 @@ func TestWorkflowHandler_Validate_ValidWorkflow_ReturnsEmptyErrors(t *testing.T) assert.Empty(t, result.Body.Errors) } + +func TestWorkflowHandler_Get_LocalScope_PassesNameOnly(t *testing.T) { + mock := newMockWorkflowLister("deploy-prod") + + api := newWorkflowHandlerAPI(t, mock) + + resp := api.Get("/api/workflows/local/deploy-prod") + require.Equal(t, 200, resp.Code) + assert.Equal(t, "deploy-prod", mock.lastGetName) +} + +func TestWorkflowHandler_Get_PackScope_PassesScopeSlashName(t *testing.T) { + mock := newMockWorkflowLister("speckit/specify") + + api := newWorkflowHandlerAPI(t, mock) + + resp := api.Get("/api/workflows/speckit/specify") + require.Equal(t, 200, resp.Code) + assert.Equal(t, "speckit/specify", mock.lastGetName) +} + +func TestWorkflowHandler_Get_UnknownWorkflow_Returns404(t *testing.T) { + mock := newMockWorkflowLister() + + api := newWorkflowHandlerAPI(t, mock) + + resp := api.Get("/api/workflows/unknown/foo") + assert.Equal(t, 404, resp.Code) +} + +func TestWorkflowHandler_Validate_PackScope_ReturnsEmptyErrors(t *testing.T) { + mock := newMockWorkflowLister("speckit/specify") + + api := newWorkflowHandlerAPI(t, mock) + + validateInput := struct { + Body struct { + Inputs map[string]any `json:"inputs"` + } `json:"body"` + }{} + + resp := api.Post("/api/workflows/speckit/specify/validate", validateInput) + require.Equal(t, 200, resp.Code) + + var result struct { + Body struct { + Errors []string `json:"errors"` + } `json:"body"` + } + err := json.NewDecoder(resp.Body).Decode(&result) + require.NoError(t, err) + + assert.Empty(t, result.Body.Errors) + assert.Equal(t, "speckit/specify", mock.lastValidateName) +} + +func TestWorkflowHandler_List_PopulatesScopeAndWorkflow(t *testing.T) { + mock := newMockWorkflowLister("local-deploy", "speckit/specify") + + api := newWorkflowHandlerAPI(t, mock) + + resp := api.Get("/api/workflows") + require.Equal(t, 200, resp.Code) + + var result struct { + Body struct { + Workflows []WorkflowSummary `json:"workflows"` + } `json:"body"` + } + err := json.NewDecoder(resp.Body).Decode(&result) + require.NoError(t, err) + + require.Len(t, result.Body.Workflows, 2) + + assert.Equal(t, "local", result.Body.Workflows[0].Scope) + assert.Equal(t, "local-deploy", result.Body.Workflows[0].Workflow) + + assert.Equal(t, "speckit", result.Body.Workflows[1].Scope) + assert.Equal(t, "specify", result.Body.Workflows[1].Workflow) +} diff --git a/internal/interfaces/api/routing.go b/internal/interfaces/api/routing.go new file mode 100644 index 00000000..54e8f23f --- /dev/null +++ b/internal/interfaces/api/routing.go @@ -0,0 +1,40 @@ +package api + +import "github.com/awf-project/cli/internal/domain/ports" + +// isRepoScope reports whether the given scope sentinel maps to one of the +// CompositeRepository's own sources (env / local / global), as opposed to a +// vendor pack name. These scopes never prefix the workflow identifier — the +// bare name is enough for WorkflowService.GetWorkflow to resolve via the +// repository, which already searches all three sources in priority order. +// +// Deriving the set from the ports.SourceXxx constants keeps the URL grammar +// in lockstep with the domain definitions; adding a new WorkflowSource will +// force a compile-time update here rather than a silent desync. +func isRepoScope(scope string) bool { + switch ports.WorkflowSource(scope) { + case ports.SourceEnv, ports.SourceLocal, ports.SourceGlobal: + return true + } + return false +} + +// recomposeIdentifier reconstructs the canonical workflow identifier from the +// (scope, name) path components of the HTTP routes. +// +// The grammar of `/api/workflows/{scope}/{name}` decomposes the identifier +// used by the application layer: +// - scope in {"local", "global", "env"} → identifier is the bare workflow +// name (e.g. "deploy-prod"); the CompositeRepository resolves it across +// its configured sources. +// - any other scope → identifier is "scope/name" (e.g. "speckit/specify"); +// this is how pack-sourced workflows are addressed by WorkflowService. +// +// This helper is the single source of truth for the URL ↔ identifier mapping +// shared by workflow and execution handlers. +func recomposeIdentifier(scope, name string) string { + if isRepoScope(scope) { + return name + } + return scope + "/" + name +} diff --git a/internal/interfaces/api/routing_test.go b/internal/interfaces/api/routing_test.go new file mode 100644 index 00000000..6137570f --- /dev/null +++ b/internal/interfaces/api/routing_test.go @@ -0,0 +1,52 @@ +package api + +import "testing" + +func TestRecomposeIdentifier(t *testing.T) { + tests := []struct { + name string + scope string + input string + expected string + }{ + { + name: "local scope returns bare name", + scope: "local", + input: "deploy-prod", + expected: "deploy-prod", + }, + { + name: "global scope returns bare name (regression: CompositeRepository resolves globally)", + scope: "global", + input: "audit", + expected: "audit", + }, + { + name: "env scope returns bare name (regression: env source is also a repo scope)", + scope: "env", + input: "custom", + expected: "custom", + }, + { + name: "pack scope composes scope/name", + scope: "speckit", + input: "specify", + expected: "speckit/specify", + }, + { + name: "arbitrary pack name composes scope/name", + scope: "acme", + input: "deploy", + expected: "acme/deploy", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := recomposeIdentifier(tt.scope, tt.input) + if got != tt.expected { + t.Errorf("recomposeIdentifier(%q, %q) = %q, want %q", tt.scope, tt.input, got, tt.expected) + } + }) + } +} diff --git a/internal/interfaces/api/server_test.go b/internal/interfaces/api/server_test.go index 48de553e..1f11a509 100644 --- a/internal/interfaces/api/server_test.go +++ b/internal/interfaces/api/server_test.go @@ -45,9 +45,9 @@ func TestServer_RegistersAllRoutes(t *testing.T) { // are excluded by using a known execution ID for execution-scoped routes. }{ {"GET", "/api/workflows"}, - {"GET", "/api/workflows/wf-1"}, - {"POST", "/api/workflows/wf-1/validate"}, - {"POST", "/api/workflows/wf-1/run"}, + {"GET", "/api/workflows/local/wf-1"}, + {"POST", "/api/workflows/local/wf-1/validate"}, + {"POST", "/api/workflows/local/wf-1/run"}, {"GET", "/api/executions"}, {"GET", "/api/executions/" + knownID}, {"DELETE", "/api/executions/" + knownID}, @@ -103,8 +103,9 @@ func TestServer_OpenAPISpec_ValidatesAgainst31(t *testing.T) { expectedPaths := []string{ "/api/workflows", - "/api/workflows/{name}", - "/api/workflows/{name}/run", + "/api/workflows/{scope}/{name}", + "/api/workflows/{scope}/{name}/run", + "/api/workflows/{scope}/{name}/validate", "/api/executions", "/api/executions/{id}", "/api/history", diff --git a/internal/interfaces/api/types.go b/internal/interfaces/api/types.go index 6cb2c4d9..0ab33e08 100644 --- a/internal/interfaces/api/types.go +++ b/internal/interfaces/api/types.go @@ -10,8 +10,14 @@ import ( // --- Workflow list --- +// WorkflowSummary is the listing DTO for GET /api/workflows entries. +// Scope and Workflow are the two components of the canonical URL grammar +// `/api/workflows/{scope}/{name}`; Name is the display identifier +// (`scope/workflow` for pack entries, plain name for local/global). type WorkflowSummary struct { Name string `json:"name" doc:"Workflow name." example:"deploy-prod"` + Scope string `json:"scope" doc:"Workflow scope (\"local\" or vendor pack name)." example:"local"` + Workflow string `json:"workflow" doc:"Workflow name without scope prefix." example:"deploy-prod"` Version string `json:"version" doc:"Workflow version." example:"1.0.0"` Description string `json:"description" doc:"Short description." example:"Deploy to production"` } @@ -29,7 +35,8 @@ type ListWorkflowsOutput struct { // --- Workflow get --- type GetWorkflowInput struct { - Name string `path:"name" doc:"Workflow name." example:"deploy-prod" required:"true"` + Scope string `path:"scope" doc:"Workflow scope (\"local\" or vendor pack name)." example:"local" required:"true"` + Name string `path:"name" doc:"Workflow name." example:"deploy-prod" required:"true"` } type GetWorkflowOutput struct { @@ -41,7 +48,8 @@ type GetWorkflowOutput struct { // --- Workflow validate --- type ValidateWorkflowInput struct { - Name string `path:"name" doc:"Workflow name." example:"deploy-prod" required:"true"` + Scope string `path:"scope" doc:"Workflow scope (\"local\" or vendor pack name)." example:"local" required:"true"` + Name string `path:"name" doc:"Workflow name." example:"deploy-prod" required:"true"` } type validateWorkflowBody struct { @@ -57,8 +65,9 @@ type ValidateWorkflowOutput struct { // --- Workflow run --- type RunWorkflowInput struct { - Name string `path:"name" doc:"Workflow name." example:"deploy-prod" required:"true"` - Body struct { + Scope string `path:"scope" doc:"Workflow scope (\"local\" or vendor pack name)." example:"local" required:"true"` + Name string `path:"name" doc:"Workflow name." example:"deploy-prod" required:"true"` + Body struct { Inputs map[string]any `json:"inputs" doc:"Key/value inputs passed to the workflow."` } } diff --git a/internal/interfaces/tui/tab_workflows.go b/internal/interfaces/tui/tab_workflows.go index 231a72b2..5a01b50c 100644 --- a/internal/interfaces/tui/tab_workflows.go +++ b/internal/interfaces/tui/tab_workflows.go @@ -29,12 +29,12 @@ type workflowItem struct { entry workflow.WorkflowEntry } -func (i workflowItem) Title() string { +func (i workflowItem) Title() string { //nolint:gocritic // value receiver required by list.Item interface; size increase from WorkflowEntry domain expansion return i.entry.Name } -func (i workflowItem) Description() string { - parts := []string{} +func (i workflowItem) Description() string { //nolint:gocritic // value receiver required by list.Item interface; size increase from WorkflowEntry domain expansion + var parts []string if i.entry.Description != "" { parts = append(parts, i.entry.Description) } @@ -53,7 +53,7 @@ func (i workflowItem) Description() string { return strings.Join(parts, " · ") } -func (i workflowItem) FilterValue() string { +func (i workflowItem) FilterValue() string { //nolint:gocritic // value receiver required by list.Item interface; size increase from WorkflowEntry domain expansion return i.entry.Name + " " + i.entry.Description + " " + i.entry.Source } @@ -239,11 +239,7 @@ func (t WorkflowsTab) updateInputForm(msg tea.Msg) (WorkflowsTab, tea.Cmd) { switch keyMsg.String() { case "esc": - t.view = workflowsListView - t.inputTarget = nil - t.inputFields = nil - t.inputNames = nil - t.inputRequired = nil + t = t.resetInputForm() return t, nil case "tab", "down": @@ -291,11 +287,7 @@ func (t WorkflowsTab) submitInputForm() (WorkflowsTab, tea.Cmd) { } wf := t.inputTarget - t.view = workflowsListView - t.inputTarget = nil - t.inputFields = nil - t.inputNames = nil - t.inputRequired = nil + t = t.resetInputForm() return t, func() tea.Msg { return LaunchWorkflowMsg{Workflow: wf, Inputs: inputs} @@ -311,11 +303,26 @@ func (t WorkflowsTab) handleValidate() (bool, WorkflowsTab, tea.Cmd) { if t.bridge != nil && t.ctx != nil { t.validating = true tick := t.spinner.Tick - return true, t, tea.Batch(t.bridge.ValidateWorkflow(t.ctx, item.wf.Name), func() tea.Msg { return tick() }) + return true, t, tea.Batch(t.bridge.ValidateWorkflow(t.ctx, item.entry.Name), func() tea.Msg { return tick() }) } return true, t, nil } +// resetInputForm clears all input form state and returns the tab to the list +// view. Called from both the Escape branch of updateInputForm (cancel) and +// from submitInputForm (after capturing the target workflow) so the two paths +// reset the same five fields identically. +// +//nolint:gocritic // Bubbletea convention: value receivers +func (t WorkflowsTab) resetInputForm() WorkflowsTab { + t.view = workflowsListView + t.inputTarget = nil + t.inputFields = nil + t.inputNames = nil + t.inputRequired = nil + return t +} + func (t WorkflowsTab) selectedWorkflowItem() (workflowItem, bool) { //nolint:gocritic // read-only selected := t.list.SelectedItem() if selected == nil { diff --git a/internal/interfaces/tui/tab_workflows_test.go b/internal/interfaces/tui/tab_workflows_test.go index 9d4216a0..e68b78e5 100644 --- a/internal/interfaces/tui/tab_workflows_test.go +++ b/internal/interfaces/tui/tab_workflows_test.go @@ -1,10 +1,12 @@ package tui import ( + "context" "fmt" "testing" "charm.land/bubbles/v2/list" + "charm.land/bubbles/v2/textinput" tea "charm.land/bubbletea/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -12,6 +14,25 @@ import ( "github.com/awf-project/cli/internal/domain/workflow" ) +// trackingWorkflowLister is a minimal WorkflowLister that records the name passed +// to ValidateWorkflow so tests can assert the correct identifier is forwarded. +type trackingWorkflowLister struct { + validatedName string +} + +func (l *trackingWorkflowLister) ListAllWorkflows(_ context.Context) ([]workflow.WorkflowEntry, error) { + return nil, nil +} + +func (l *trackingWorkflowLister) GetWorkflow(_ context.Context, _ string) (*workflow.Workflow, error) { + return nil, nil +} + +func (l *trackingWorkflowLister) ValidateWorkflow(_ context.Context, name string) error { + l.validatedName = name + return nil +} + // --- workflowItem --- func TestWorkflowItem_Title_ReturnsWorkflowName(t *testing.T) { @@ -482,3 +503,78 @@ func TestWorkflowItem_ImplementsListDefaultItem(t *testing.T) { item := workflowItem{entry: entry} var _ list.DefaultItem = item } + +// --- FIX #1: handleValidate nil-pointer guard for pack workflows --- + +// TestWorkflowsTab_handleValidate_PackWorkflow_NoPanic verifies that calling +// handleValidate on a pack-sourced workflow entry (where wf == nil because the +// wfMap lookup misses on the fully-qualified "packName/workflowName" key) does +// not panic and forwards the fully-qualified entry name to ValidateWorkflow. +func TestWorkflowsTab_handleValidate_PackWorkflow_NoPanic(t *testing.T) { + tab := newWorkflowsTab() + + // Pack entries have Name == "packName/workflowName"; the bare workflow + // struct only carries the plain name, so setWorkflows wfMap lookup misses + // and item.wf ends up nil for this entry. + entries := []workflow.WorkflowEntry{ + {Name: "speckit/specify", Source: "pack", Scope: "speckit", Workflow: "specify"}, + } + wfs := []*workflow.Workflow{ + {Name: "specify"}, + } + tab.setWorkflows(entries, wfs) + tab, _ = tab.Update(tea.WindowSizeMsg{Width: 80, Height: 24}) + + lister := &trackingWorkflowLister{} + bridge := NewBridge(lister, nil, nil) + tab.bridge = bridge + tab.ctx = context.Background() + + var handled bool + var cmd tea.Cmd + require.NotPanics(t, func() { + handled, tab, cmd = tab.handleValidate() + }) + + assert.True(t, handled) + assert.True(t, tab.validating, "tab must enter validating state when bridge is set") + require.NotNil(t, cmd, "handleValidate must return a non-nil command when bridge is set") + + // handleValidate composes its work via tea.Batch, whose returned Cmd yields a + // tea.BatchMsg containing the sub-Cmds — sub-Cmds are not executed until the + // Bubbletea runtime drains the batch. Drain manually so the validation Cmd + // runs and the lister records the name it received. + msg := cmd() + batch, ok := msg.(tea.BatchMsg) + require.True(t, ok, "handleValidate must return a tea.BatchMsg, got %T", msg) + for _, sub := range batch { + // Sub-Cmds may themselves return BatchMsgs in deeper batching; the + // validation Cmd from Bridge.ValidateWorkflow returns a single Msg + // directly, so a flat iteration suffices here. + _ = sub() + } + + assert.Equal(t, "speckit/specify", lister.validatedName, + "ValidateWorkflow must receive the fully-qualified entry name, not the bare workflow name") +} + +// --- FIX #2: resetInputForm clears all form state --- + +// TestWorkflowsTab_resetInputForm_ClearsAllFields verifies that resetInputForm +// resets the view back to the list and nils all four input form fields. +func TestWorkflowsTab_resetInputForm_ClearsAllFields(t *testing.T) { + tab := newWorkflowsTab() + tab.view = workflowsInputView + tab.inputTarget = &workflow.Workflow{Name: "x"} + tab.inputFields = make([]textinput.Model, 1) + tab.inputNames = []string{"param"} + tab.inputRequired = []bool{true} + + tab = tab.resetInputForm() + + assert.Equal(t, workflowsListView, tab.view, "view must be reset to workflowsListView") + assert.Nil(t, tab.inputTarget, "inputTarget must be nil after reset") + assert.Nil(t, tab.inputFields, "inputFields must be nil after reset") + assert.Nil(t, tab.inputNames, "inputNames must be nil after reset") + assert.Nil(t, tab.inputRequired, "inputRequired must be nil after reset") +} diff --git a/internal/testutil/mocks/mocks.go b/internal/testutil/mocks/mocks.go index b0edf735..1a53ffb8 100644 --- a/internal/testutil/mocks/mocks.go +++ b/internal/testutil/mocks/mocks.go @@ -114,6 +114,25 @@ func (m *MockWorkflowRepository) List(ctx context.Context) ([]string, error) { return names, nil } +// ListWithSource returns all workflow names together with their discovery source. +// All entries report SourceLocal since the mock does not distinguish origins. +// Thread-safe for concurrent access. +func (m *MockWorkflowRepository) ListWithSource(ctx context.Context) ([]ports.WorkflowInfo, error) { + m.mu.RLock() + defer m.mu.RUnlock() + + if m.listErr != nil { + return nil, m.listErr + } + + infos := make([]ports.WorkflowInfo, 0, len(m.workflows)) + for name := range m.workflows { + infos = append(infos, ports.WorkflowInfo{Name: name, Source: ports.SourceLocal}) + } + + return infos, nil +} + // Exists checks if a workflow with the given name exists. // Thread-safe for concurrent access. func (m *MockWorkflowRepository) Exists(ctx context.Context, name string) (bool, error) { diff --git a/tests/fixtures/api/packs/speckit/manifest.yaml b/tests/fixtures/api/packs/speckit/manifest.yaml new file mode 100644 index 00000000..bc542dc0 --- /dev/null +++ b/tests/fixtures/api/packs/speckit/manifest.yaml @@ -0,0 +1,8 @@ +name: speckit +version: "1.0.0" +description: Specification kit workflow pack for integration testing +author: test +license: MIT +awf_version: ">=0.5.0" +workflows: + - specify diff --git a/tests/fixtures/api/packs/speckit/state.json b/tests/fixtures/api/packs/speckit/state.json new file mode 100644 index 00000000..c4fa9e73 --- /dev/null +++ b/tests/fixtures/api/packs/speckit/state.json @@ -0,0 +1 @@ +{"name":"speckit","enabled":true,"source_data":{"repository":"test/speckit","version":"1.0.0"}} diff --git a/tests/fixtures/api/packs/speckit/workflows/specify.yaml b/tests/fixtures/api/packs/speckit/workflows/specify.yaml new file mode 100644 index 00000000..ff789908 --- /dev/null +++ b/tests/fixtures/api/packs/speckit/workflows/specify.yaml @@ -0,0 +1,16 @@ +name: specify +description: Minimal specify workflow for integration testing +version: "1.0.0" +author: test + +states: + initial: run + run: + type: step + command: echo "specify" + on_success: done + on_failure: failed + done: + type: terminal + failed: + type: terminal diff --git a/tests/integration/api/functional_test.go b/tests/integration/api/functional_test.go index fc194652..002dd494 100644 --- a/tests/integration/api/functional_test.go +++ b/tests/integration/api/functional_test.go @@ -53,7 +53,7 @@ func TestAPI_GetWorkflow_Integration(t *testing.T) { ts, _, _ := newTestServer(t, apiFixtureDir(t)) req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, - ts.URL+"/api/workflows/api-simple-success", nil) + ts.URL+"/api/workflows/local/api-simple-success", nil) require.NoError(t, err) resp, err := http.DefaultClient.Do(req) @@ -79,7 +79,7 @@ func TestAPI_GetWorkflow_NotFound_Integration(t *testing.T) { ts, _, _ := newTestServer(t, apiFixtureDir(t)) req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, - ts.URL+"/api/workflows/nonexistent-workflow", nil) + ts.URL+"/api/workflows/local/nonexistent-workflow", nil) require.NoError(t, err) resp, err := http.DefaultClient.Do(req) @@ -213,7 +213,7 @@ func TestAPI_RunWorkflow_InvalidInputs_Integration(t *testing.T) { require.NoError(t, err) req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, - ts.URL+"/api/workflows/api-simple-success/run", bytes.NewReader(bodyBytes)) + ts.URL+"/api/workflows/local/api-simple-success/run", bytes.NewReader(bodyBytes)) require.NoError(t, err) req.Header.Set("Content-Type", "application/json") diff --git a/tests/integration/api/pack_workflow_test.go b/tests/integration/api/pack_workflow_test.go new file mode 100644 index 00000000..1addc8e6 --- /dev/null +++ b/tests/integration/api/pack_workflow_test.go @@ -0,0 +1,298 @@ +//go:build integration + +package api_test + +import ( + "bufio" + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/awf-project/cli/internal/application" + "github.com/awf-project/cli/internal/infrastructure/executor" + infraExpr "github.com/awf-project/cli/internal/infrastructure/expression" + "github.com/awf-project/cli/internal/infrastructure/repository" + "github.com/awf-project/cli/internal/infrastructure/store" + "github.com/awf-project/cli/internal/infrastructure/workflowpkg" + "github.com/awf-project/cli/internal/interfaces/api" + "github.com/awf-project/cli/pkg/interpolation" + "github.com/awf-project/cli/tests/integration/testhelpers" +) + +// newTestServerWithPacks builds an httptest.Server wired with both a local +// YAML repository and a PackDiscoverer rooted at packsDir. +func newTestServerWithPacks(t *testing.T, fixtureDir, packsDir string) *httptest.Server { + t.Helper() + + statesDir := t.TempDir() + logger := &testhelpers.MockLogger{} + + repo := repository.NewYAMLRepository(fixtureDir) + stateStore := store.NewJSONStore(statesDir) + shellExec := executor.NewShellExecutor() + resolver := interpolation.NewTemplateResolver() + evaluator := infraExpr.NewExprEvaluator() + validator := infraExpr.NewExprValidator() + + wfSvc := application.NewWorkflowService(repo, stateStore, shellExec, logger, validator) + wfSvc.SetPackDiscoverer(workflowpkg.NewPackDiscovererAdapter([]string{packsDir})) + + parallelExec := application.NewParallelExecutor(logger) + execSvc := application.NewExecutionServiceWithEvaluator( + wfSvc, shellExec, parallelExec, stateStore, logger, resolver, nil, evaluator, + ) + + bridge := api.NewBridge(wfSvc, execSvc, nil) + srv := api.NewServer(bridge, "127.0.0.1:0") + ts := httptest.NewServer(srv.Handler()) + t.Cleanup(ts.Close) + + return ts +} + +// packFixtureDir returns the path to tests/fixtures/api/packs relative to the repo root. +func packFixtureDir(t *testing.T) string { + t.Helper() + return filepath.Join(testhelpers.GetRepoRoot(t), "tests", "fixtures", "api", "packs") +} + +// assertPackWiring fails fast if GET /api/workflows does not return at least one +// entry with scope=="speckit" and one with scope=="local". +func assertPackWiring(t *testing.T, ts *httptest.Server) { + t.Helper() + + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, ts.URL+"/api/workflows", nil) + require.NoError(t, err) + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + require.Equal(t, http.StatusOK, resp.StatusCode) + + var result struct { + Body struct { + Workflows []struct { + Scope string `json:"scope"` + } `json:"workflows"` + } `json:"body"` + } + require.NoError(t, json.NewDecoder(resp.Body).Decode(&result)) + + var hasSpeckit, hasLocal bool + for _, w := range result.Body.Workflows { + if w.Scope == "speckit" { + hasSpeckit = true + } + if w.Scope == "local" { + hasLocal = true + } + } + require.True(t, hasSpeckit, "PackDiscoverer wiring failed: no speckit-scoped workflow in GET /api/workflows") + require.True(t, hasLocal, "local repository wiring failed: no local-scoped workflow in GET /api/workflows") +} + +// TestAPI_PackWorkflow_Get sends GET /api/workflows/speckit/specify and asserts 200. +func TestAPI_PackWorkflow_Get(t *testing.T) { + ts := newTestServerWithPacks(t, apiFixtureDir(t), packFixtureDir(t)) + assertPackWiring(t, ts) + + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, + ts.URL+"/api/workflows/speckit/specify", nil) + require.NoError(t, err) + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusOK, resp.StatusCode) + + var result struct { + Body struct { + Name string `json:"name"` + } `json:"body"` + } + require.NoError(t, json.NewDecoder(resp.Body).Decode(&result)) + assert.NotEmpty(t, result.Body.Name, "response body should contain workflow data") +} + +// TestAPI_PackWorkflow_LocalGet sends GET /api/workflows/local/api-simple-success and asserts 200. +func TestAPI_PackWorkflow_LocalGet(t *testing.T) { + ts := newTestServerWithPacks(t, apiFixtureDir(t), packFixtureDir(t)) + assertPackWiring(t, ts) + + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, + ts.URL+"/api/workflows/local/api-simple-success", nil) + require.NoError(t, err) + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, http.StatusOK, resp.StatusCode) +} + +// TestAPI_PackWorkflow_Run sends POST /api/workflows/speckit/specify/run and polls until completed. +func TestAPI_PackWorkflow_Run(t *testing.T) { + ts := newTestServerWithPacks(t, apiFixtureDir(t), packFixtureDir(t)) + assertPackWiring(t, ts) + + bodyBytes, err := json.Marshal(map[string]any{"inputs": map[string]any{}}) + require.NoError(t, err) + + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, + ts.URL+"/api/workflows/speckit/specify/run", bytes.NewReader(bodyBytes)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusAccepted, resp.StatusCode) + + var runResult struct { + Body struct { + ExecutionID string `json:"execution_id"` + } `json:"body"` + } + require.NoError(t, json.NewDecoder(resp.Body).Decode(&runResult)) + executionID := runResult.Body.ExecutionID + require.NotEmpty(t, executionID, "execution_id must be present in run response") + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + ticker := time.NewTicker(200 * time.Millisecond) + defer ticker.Stop() + + for { + select { + case <-ctx.Done(): + t.Fatal("pack workflow execution did not complete within timeout") + case <-ticker.C: + pollReq, pollErr := http.NewRequestWithContext(context.Background(), http.MethodGet, + ts.URL+"/api/executions/"+executionID, nil) + require.NoError(t, pollErr) + + pollResp, pollErr := http.DefaultClient.Do(pollReq) + require.NoError(t, pollErr) + + var pollResult struct { + Body struct { + Status string `json:"status"` + } `json:"body"` + } + json.NewDecoder(pollResp.Body).Decode(&pollResult) //nolint:errcheck + pollResp.Body.Close() + + if pollResult.Body.Status == "completed" || pollResult.Body.Status == "failed" { + assert.Equal(t, "completed", pollResult.Body.Status, "pack workflow execution should reach completed status") + return + } + } + } +} + +// TestAPI_PackWorkflow_Validate sends POST /api/workflows/speckit/specify/validate and asserts 200 with no errors. +func TestAPI_PackWorkflow_Validate(t *testing.T) { + ts := newTestServerWithPacks(t, apiFixtureDir(t), packFixtureDir(t)) + assertPackWiring(t, ts) + + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, + ts.URL+"/api/workflows/speckit/specify/validate", nil) + require.NoError(t, err) + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusOK, resp.StatusCode) + + var result struct { + Body struct { + Errors []string `json:"errors"` + } `json:"body"` + } + require.NoError(t, json.NewDecoder(resp.Body).Decode(&result)) + assert.Empty(t, result.Body.Errors, "valid workflow should produce no validation errors") +} + +// TestAPI_PackWorkflow_SSE runs the speckit/specify workflow and asserts the SSE stream +// terminates with workflow.completed or workflow.failed before a 5-second deadline. +func TestAPI_PackWorkflow_SSE(t *testing.T) { + ts := newTestServerWithPacks(t, apiFixtureDir(t), packFixtureDir(t)) + assertPackWiring(t, ts) + + bodyBytes, err := json.Marshal(map[string]any{"inputs": map[string]any{}}) + require.NoError(t, err) + + runReq, err := http.NewRequestWithContext(context.Background(), http.MethodPost, + ts.URL+"/api/workflows/speckit/specify/run", bytes.NewReader(bodyBytes)) + require.NoError(t, err) + runReq.Header.Set("Content-Type", "application/json") + + runResp, err := http.DefaultClient.Do(runReq) + require.NoError(t, err) + defer runResp.Body.Close() + require.Equal(t, http.StatusAccepted, runResp.StatusCode) + + var runResult struct { + Body struct { + ExecutionID string `json:"execution_id"` + } `json:"body"` + } + require.NoError(t, json.NewDecoder(runResp.Body).Decode(&runResult)) + executionID := runResult.Body.ExecutionID + require.NotEmpty(t, executionID) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + sseReq, err := http.NewRequestWithContext(ctx, http.MethodGet, + ts.URL+"/api/executions/"+executionID+"/events", nil) + require.NoError(t, err) + sseReq.Header.Set("Accept", "text/event-stream") + + sseResp, err := http.DefaultClient.Do(sseReq) + require.NoError(t, err) + defer sseResp.Body.Close() + require.Equal(t, http.StatusOK, sseResp.StatusCode) + + var terminalEventType string + scanner := bufio.NewScanner(sseResp.Body) + var currentType string + for scanner.Scan() { + select { + case <-ctx.Done(): + t.Fatal("SSE stream did not emit a terminal event within 5-second deadline") + default: + } + line := scanner.Text() + switch { + case strings.HasPrefix(line, "event: "): + currentType = strings.TrimPrefix(line, "event: ") + case line == "" && currentType != "": + if currentType == "workflow.completed" || currentType == "workflow.failed" { + terminalEventType = currentType + goto done + } + currentType = "" + } + } +done: + assert.NotEmpty(t, terminalEventType, "SSE stream should have emitted at least one terminal event") + assert.True( + t, + terminalEventType == "workflow.completed" || terminalEventType == "workflow.failed", + "terminal SSE event type should be workflow.completed or workflow.failed, got: %s", terminalEventType, + ) +} diff --git a/tests/integration/api/server_integration_test.go b/tests/integration/api/server_integration_test.go index 10546cc9..21f215b3 100644 --- a/tests/integration/api/server_integration_test.go +++ b/tests/integration/api/server_integration_test.go @@ -118,7 +118,7 @@ func postRunWorkflow(t *testing.T, ts *httptest.Server, name string, inputs map[ require.NoError(t, err) req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, - ts.URL+"/api/workflows/"+name+"/run", bytes.NewReader(bodyBytes)) + ts.URL+"/api/workflows/local/"+name+"/run", bytes.NewReader(bodyBytes)) require.NoError(t, err) req.Header.Set("Content-Type", "application/json")