Skip to content

fix(swarm): normalize health check test commands - #5205

Open
bestmaa wants to merge 4 commits into
Dokploy:canaryfrom
bestmaa:fix/5171-swarm-healthcheck-serialization
Open

fix(swarm): normalize health check test commands#5205
bestmaa wants to merge 4 commits into
Dokploy:canaryfrom
bestmaa:fix/5171-swarm-healthcheck-serialization

Conversation

@bestmaa

@bestmaa bestmaa commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

What is this PR about?

This fixes Swarm health checks getting stuck in Starting when the UI stores the entire Test JSON array as one string. Health-check commands are now normalized both when the form is saved and again when the Docker service spec is generated, so previously stored malformed values are covered too.

  • A JSON array pasted into one field is parsed into Docker exec form.
  • A single command is converted to CMD-SHELL form.
  • A legacy blank value is converted to NONE instead of [""].
  • Invalid command arrays are rejected with a clear message before they reach Docker.
  • The form description now explains the supported input styles.

Testing

  • pnpm --filter=dokploy exec vitest --config __test__/vitest.config.ts __test__/server/swarm-health-check.test.ts __test__/server/mechanizeDockerContainer.test.ts --run: 20 tests passed on Node 24.4.0.
  • pnpm --filter=@dokploy/server run typecheck: passed.
  • pnpm build: passed on Node 24.4.0.
  • Docker 29.7.2 isolated Swarm reproduction:
    • Before: Test: [""] stayed at 0/1 and Docker logged an unknown healthcheck type warning.
    • After: pasted JSON, blank/disabled, and single-shell-command cases all reached 1/1 Running, with no new invalid-healthcheck warning.

Note: the current canary baseline has unrelated formatting and type errors in write-app-traefik-config.test.ts introduced by #5202; a test-only fix is in #5209. The server package, changed files, focused tests, and production build pass.

Checklist

  • Created a dedicated branch based on the latest canary branch.
  • Read and followed the contribution guidelines.
  • Tested the change locally with the repository Node version and an isolated Docker Swarm.

Issues related

Closes #5171

Screenshots

Not applicable; this change is covered by before/after Docker service state and regression tests.

Greptile Summary

This PR adds shared normalization for Swarm health-check commands at both form-save and Docker service-generation boundaries.

  • Parses pasted JSON-array commands and normalizes single commands into shell form
  • Handles blank and disabled health checks
  • Rejects unsupported command shapes and adds focused regression coverage
  • Updates the dashboard guidance and error reporting

Confidence Score: 4/5

The PR should not merge until pasted JSON arrays beginning with non-string values are rejected instead of being persisted as shell health checks.

The new JSON detection only validates a subset of array-shaped inputs, allowing inputs such as [1] to bypass the advertised array-of-strings validation and reach Docker as an unintended shell command.

Files Needing Attention: packages/server/src/utils/docker/health-check.ts

Reviews (1): Last reviewed commit: "fix(swarm): normalize health check test ..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

@bestmaa
bestmaa requested a review from Siumauricio as a code owner August 28, 2026 12:54
@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Aug 28, 2026
return ["NONE"];
}

if (/^\[\s*(?:"|\])/.test(value)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 JSON arrays bypass validation

When a user pastes a JSON array beginning with a non-string value such as [1], this predicate skips JSON parsing and converts the input to CMD-SHELL, causing Docker to execute the JSON text as a shell health check instead of returning the intended validation error.

Suggested change
if (/^\[\s*(?:"|\])/.test(value)) {
if (value.startsWith("[")) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1727c27. The normalizer now parses every value beginning with an opening bracket before validating the array entries, while still preserving valid POSIX test expressions such as [ -f /tmp/ready ] as shell commands. I also added regression coverage for [1]; the focused suite now passes all 20 tests. Thanks for catching this.

@bestmaa

bestmaa commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

The red checks here are caused by existing canary type errors in the Traefik regression tests from #5202, not by the Swarm health-check changes. I opened #5209 as a small test-only follow-up. Once it lands, I will update this branch from canary and rerun the checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Swarm Settings health check 'Test' field serializes as single string instead of JSON array, causing tasks to hang in 'Starting' forever

1 participant