* feat(rest): add task signal endpoints
Adds the task signal endpoints the Go SDK / CLI already call but OSS never
implemented (conductor-oss/conductor#1197):
POST /api/tasks/{workflowId}/{status}/signal (async)
POST /api/tasks/{workflowId}/{status}/signal/sync (sync, returns SignalResponse)
Before this, the SDK's SignalAsync hit POST /tasks/{wfId}/{status}/signal, which
had no matching route, so Spring fell through to the all-variable update route
/{workflowId}/{taskRefName}/{status} and tried to coerce the literal "signal"
into TaskResult.Status -> MethodArgumentTypeMismatchException. Adding the literal
/signal segment makes that pattern more specific, so it now wins; a MockMvc
routing test (with PathPatternParser, mirroring production) locks this in.
"Signal" finds the first non-terminal WAIT task in the workflow (descending into
running sub-workflows) and applies the given status + output to it, via the new
TaskService.signalTask. The sync variant then waits for the workflow to settle
into its next blocking/terminal state and renders a SignalResponse per the
returnStrategy, reusing the same poll-to-response logic as executeWorkflow --
extracted into WorkflowSignalResponder so both controllers share it.
Tests: TaskServiceTest (signalTask found / not-found), TaskResourceTest (async,
sync, not-found, route resolution). Full conductor-rest suite green.
* fix(signal): add signalTimeout field and E2E Groovy integration tests
Two issues raised in PR #1205 review:
1. `SignalResponse` was missing the `signalTimeout` boolean that Orkes sets
when the sync-signal poll times out. Without it a timed-out response looks
identical to a successful one. Added `signalTimeout` to `SignalResponse`
(absorbed by `WorkflowRun` and `TaskRun` via inheritance), propagated it
through `NotificationResult.toResponse()`, and set it to `true` in
`WorkflowSignalResponder`'s timeout fallback path.
2. Added `SignalTaskSpec` to the test-harness — a Spring Boot integration test
backed by a real Redis testcontainer that exercises `TaskService.signalTask()`
without any mocking: direct-WAIT-task signal, signal-with-no-blocker,
signal-on-nonexistent-workflow, and sub-workflow descent.
* test(signal): remove mocked signal tests — covered by SignalTaskSpec E2E
* fix(test): SignalTaskSpec — expect NotFoundException for missing workflow
ExecutionDAOFacade.getWorkflow() throws NotFoundException for unknown IDs
(does not return null). The test was asserting null — corrected to thrown().
The e2e WorkflowRerunTests failure in the same CI run is pre-existing and
unrelated to signal changes (it was already failing on the prior commit).
* test(e2e): HTTP-level signal endpoint tests (async + sync)
* fix: spotless formatting violations in SignalTaskTest
Workflow names containing '/' or '%' were accepted by the name validator
but could never be reopened: the name is carried as a path segment
(/workflowDef/{name} and GET /metadata/workflow/{name}), and the
percent-encoded %2F / %25 is rejected by the server's path handling (400),
leaving the definition saved but unreachable with only a broken editor
shell.
Drop '%' from WORKFLOW_NAME_REGEX ('/' was already excluded) so both
unreachable characters fail client-side validation at edit time with the
standard name-error message, instead of persisting an unopenable
definition.
Fixesorkes-io/conductor-ui#4241
When a confirm-save flow closes (e.g. after the backend rejects a task-def
save), the diff editor unmounts and Monaco disposes its text models while
the DiffEditorWidget still references them, throwing an uncaught
'TextModel got disposed before DiffEditorWidget model got reset'.
Set keepCurrentOriginalModel/keepCurrentModifiedModel on the shared
DiffEditor so the models are not disposed on unmount. Applies to every
confirm-save diff view (task def, workflow def, event handler, scheduler).
Fixesorkes-io/conductor-ui#4250
A leading or trailing space in a task reference name bypassed the
duplicate-name check: the graph compared raw node ids, so " wait_ref"
and "wait_ref" were treated as distinct and Save proceeded, persisting a
ref name that looks identical in the field but silently breaks
${ref.output} references and JOINs.
Compare reference names trimmed of surrounding whitespace so
whitespace-only variants are detected as duplicates.
Fixesorkes-io/conductor-ui#4248
Result counters were hard-coded to '${n} results', so a table or search
with exactly one row read '1 results'. Add a pluralizeResults() helper
that singularizes the noun, use it in createTableTitle (shared by the
Environment Variables, Workflow/Task Definition, and Task List tables)
and in the Workflow/Task/Agent search headers and Add-task sidebar.
Fixesorkes-io/conductor-ui#4246
Queue wait time (startTime - scheduledTime) was passed through raw, so it
rendered as a unitless signed integer and could show a negative value
(e.g. -6) from sub-millisecond clock skew, directly under the properly
formatted Duration row.
Clamp to >= 0 and render via durationRenderer so it reads consistently
with Duration (e.g. '0ms'). Also guard on _isFinite so an explicit 0 is
still shown.
Fixesorkes-io/conductor-ui#4245
Multi-word task/workflow statuses rendered as raw enums with only the
first letter capitalized and the underscore kept (e.g. 'In_progress',
'Timed_out') because the status badges lowercased the value then applied
lodash capitalize, which neither replaces underscores nor title-cases
each word.
Add a humanizeStatus() helper that splits on underscores and title-cases
each word, and use it in StatusBadge, WorkflowStatusBadge, and
StatusTagChip. Single-word statuses are unchanged.
Fixesorkes-io/conductor-ui#4240
On a PR comment of exactly "/update-snapshots" (once on default branch)
or via manual workflow_dispatch against any branch, regenerates the
Playwright reference snapshots in the same pinned image the e2e-mocked
check uses (docker-compose.snapshots.yml) and commits them back to the
branch. The workflow_dispatch path is the fallback while this file has
not yet merged to main (issue_comment workflows only run from default).
The post-restart state-transition assertions used tight atMost(5s)/(10s)
awaitility windows that flake under CI load (expected SCHEDULED but was
IN_PROGRESS; sub-workflow id still null). Widen the sub-workflow-id waits
to 20s and the terminal/restart-state waits to 15s, and add a poll
interval to the block that lacked one. Behavior-preserving.
The five AgentTaskTests that assert a successful LLM completion
(helloWorld, longRunning, agentClientStartsWaitsResponds,
twoAgentConversation, concurrentCalls) ran unconditionally against a CI
server with no LLM integration configured, so agents returned blank /
FAILED. Mirror LLMChatCompleteTests by gating them behind OPENAI_API_KEY
so they skip in a keyless CI and run when a key is present. Also:
- fix the model default case (openai -> OpenAI) so it resolves against a
case-sensitively-registered integration when a key is supplied;
- relax the longRunning callback upper bound 8s -> 10s (CI-load flake).
The seven negative-path tests (FAILED/CANCELED/TIMED_OUT/NotFound) are
left ungated - they need no successful LLM and already pass.
The two page.screenshot({path:"/private/tmp/..."}) calls in the agent
metadata spec are debug dumps to a macOS-only path; on the Linux CI
runner /private/tmp does not exist, so the test throws ENOENT after its
real assertions have already passed. Remove them.
The AgentExecution status mappers only special-cased the literal "FAILED"
task status. Conductor emits several terminal-failure statuses
(FAILED_WITH_TERMINAL_ERROR, TIMED_OUT, CANCELED); these fell through the
mapTaskStatus/taskSuccess default to RUNNING/undefined, so a failed agent
LLM task rendered a perpetual "running" chip + spinner while the
workflow itself was already FAILED.
- mapTaskStatus: map all terminal-failure statuses to FAILED, restrict
RUNNING to genuinely-active statuses (IN_PROGRESS/SCHEDULED/PENDING),
and default unknown statuses to FAILED so no unmapped status can
render a perpetual spinner again.
- taskSuccess: same terminal-vs-active classification; only active
statuses return undefined (spinner), everything else terminal.
- Extract isFailedTaskStatus() and use it at the three remaining
status === "FAILED" tool/iteration sites.
- Add agentExecutionUtils unit tests covering every status.
Fixesorkes-io/conductor-ui#4260
LLM Task in workflow definition with both topP and temperature errors for claude and should be null if not defined
* Check clearEmptyNumberAsNull first
Relocates SecretsDAO out of the legacy com.netflix.conductor.dao package,
adapts SkillMetadataDAO implementations accordingly, and removes the
AgentSpan embedded environment post-processor, principal filter, and
env-backed credential store in favor of a simpler configuration wired
directly through application.properties (agentspan.embedded).
The decide(String) re-queue on a lock miss is sufficient to fix the
multi-minute JOIN-boundary pause: the existing due-based sweeper picks
up the short-backoff entry once the lock frees, and decide() invoked
from the sweeper is reentrant (sweep() already holds the lock) so it
never hits the lock-miss branch anyway. The re-queue only ever fires
from the completion-event callers (updateTask, AsyncSystemTaskExecutor)
- exactly the path that was losing the wake-up.
Verified end-to-end: DynamicForkJoinLockContentionSpec passes with the
decide() fix and WorkflowSweeper reverted to its pre-PR form.
Revert WorkflowSweeper.java and WorkflowSweeperTest.java to main; update
the design doc accordingly.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Moves the fix to the correct layer per review feedback (thanks @manan164).
The lost wake-up is decide() returning null on a workflow-lock miss: the
completion-event callers (updateTask, AsyncSystemTaskExecutor) ignore that
null, so the next task is never scheduled and the workflow parks on its
decider-queue entry — which a polled task postpones out to
responseTimeoutSeconds (e.g. 600s), the observed pause. The previous
sweeper-only change could not help this: the sweeper's sweep() runs only when
the decider entry is popped, and that entry isn't due for responseTimeoutSeconds.
- WorkflowExecutorOps.decide(String): on a lock miss, re-queue the workflow to
DECIDER_QUEUE with a lockTimeToTry/2 backoff (contention-scale, not the
lockLeaseTime scale used for orphaned locks) before returning null. This is
the primary fix and benefits every caller (completion events + sweeper).
- WorkflowSweeper.sweep(): keep the top-level lock-miss re-queue as a backstop
but at the same short backoff; drop the redundant decide()==null re-queue
(decide() now owns it); remove the lockLeaseTime-based helper.
Tests:
- TestWorkflowExecutor.testDecideReQueuesWorkflowOnLockMiss: decide() lock miss
pushes to DECIDER_QUEUE with the short backoff and returns null (fails against
the old bare return).
- DynamicForkJoinLockContentionSpec: rewritten as a true reproduction — drive a
real dynamic fork/join to the JOIN boundary, hold the workflow lock from a
foreign thread, run the JOIN (post-completion decide misses the lock), then
release the lock and assert the workflow recovers on its own within seconds via
the real background sweeper (no manual sweep, no manufactured queue state).
Fails (times out) without the decide() fix; the no-contention control passes in
both. Verified: with the fix reverted, both the unit test and this spec fail.
- Scrubbed customer identifiers from tests; dropped the three legacy
(deprecated, off-by-default) TestWorkflowSweeper cases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Viren Baraiya