Skip to content

Quality fixes for build, publish and GitHub actions - #481

Open
3alpha wants to merge 15 commits into
masterfrom
3alpha/quality-fixes
Open

3alpha wants to merge 15 commits into
masterfrom
3alpha/quality-fixes

Conversation

@3alpha

@3alpha 3alpha commented Nov 4, 2026

Copy link
Copy Markdown
Member

Summary

Fixes a set of correctness, safety and usability problems in build, publish and the GitHub actions, and modernises the toolchain. Each commit covers one theme and has regression tests. The commits are meant to be reviewed one by one.

Commits

  1. Modernise toolchain and dependencies.
    • Upgrades got 14, octokit 22, ethers 6.17, image-size 2, inquirer 12, form-data 4, tar-fs 3 and dotenv 16.
    • Drops @dappnode/toolkit, dockerode, graphql, request and rimraf. yarn audit now reports 0 advisories.
    • Replaces the toolkit APM client with a minimal local ApmRepository, whose live mainnet results match the toolkit's.
    • Moves to ESLint 10 (flat config; lint no longer rewrites files) and Mocha 12 with tsx.
    • CI uses frozen installs and a Node 20/22/24/26 matrix.
    • The live Docker/IPFS/Ethereum tests move to yarn test:integration, run by a separate non-blocking CI job.
  2. Parse --timeout predictably. Durations such as 20min, and bare integer strings as seconds (the documented --timeout 5000). Numeric 0 means no timeout, and invalid values are rejected.
  3. Run git and docker commands without a shell. Argument arrays go through execFile, so upstream tags and branch names are passed literally and can't inject shell commands. Upstream tags may contain @ (n8n).
  4. Render build progress with one bounded terminal preview.
    • Numbered package and architecture stages, with the last log lines shown under each.
    • --skip_save no longer reports a successful build as an error, and --all_variants prints the resolved variant names.
    • The build and publish handlers forward delete_old_pins, which used to be ignored.
  5. Export images atomically and for the requested platform.
    • A failed or killed docker save/xz used to be cached and uploaded as an empty archive; now it fails the build.
    • The compose path pulls external images for the requested architecture.
    • The architecture check inspects that platform (--platform, with a fallback for Docker CLI < 28.1).
  6. Confirm before init overwrites package files. The overwrite prompt was not awaited, so files were written before the user answered. An existing Dockerfile is kept unless --force is given.
  7. Package exactly what was validated for templates.
    • Release files come from the same merged root and variant configuration that passed validation.
    • Setup wizards are accepted as JSON or YAML, validated against the schema and emitted as JSON. Notification endpoints are merged by correlationId and wizard fields by id; an array in a variant replaces the root's.
    • Single files (Prometheus targets, avatar, disclaimer, …) come from the variant directory when it has one.
    • Values coerced by schema validation are listed in the "Validate files" step.
    • A single YAML manifest is accepted.
    • Publish keeps external upstream image references instead of retagging them, so the next publish can still pull them.
    • The README documents the root and variant file rules.
  8. Only unpin releases that were actually replaced.
    • Old pins are deleted only after the replacement upload succeeded, and only for ancestor commits. On a shallow CI checkout the missing history is fetched first.
    • Branch cleanup paginates GitHub branches and Pinata pins, and rechecks each branch before unpinning.
  9. Show the pinning phase and use an idle timeout for uploads.
    • Once all bytes are sent, uploads show the time spent waiting for pinning confirmation.
    • Requests abort after 15 minutes without activity, not at a fixed total, so slow but healthy uploads of large releases are not cut off.
    • HTTP/2 for IPFS uploads (Bump got, enforce http2 #466) is kept.
    • The Swarm upload URL works with prefixUrl.
  10. Complete GitHub releases and honour --dir in GitHub actions.
    • Releases upload every optional asset; setup wizards and dashboards were silently missing before.
    • Upload errors fail the publish. An asset that a failed attempt already stored completely is accepted, and a partial one is replaced.
    • GitHub action and bump-upstream builds pass --dir, the variants directory and the compose file name.
  11. Report failed commands once and log each line once.
    • A failed build printed its command output three or four times and ended in a Node crash dump. It now prints one error block and exits with code 1.
    • Verbose and CI logs print each build line once.

Behaviour changes worth a look

  • Template packages: variant files win. For single release files, a variant's copy now replaces the root's. Previously only the root's was used, except for Prometheus targets and notifications, where the variant already won.
  • Released notifications and wizards are the validated objects. Values coerced by the schema ship, for example DMS's isBanner: "false" becomes false, and a warning lists every coerced value.
  • delete_old_pins now actually deletes, and only replaced ancestor pins.
  • GitHub release upload errors fail the publish. They used to be logged and ignored.
  • Uploads no longer wait forever on a stalled connection. They abort after 15 minutes with no activity.

Verification

  • yarn lint, yarn build and yarn test pass (144 passing, 1 pending). The type check and tests pass at every commit; the stricter lint config from the first commit passes once the later fixes are applied.
  • All 97 non-archived dappnode/DAppNodePackage* repos were run through validation, release-dir creation and file copy with the old and new code:
    • Every repo has the same pass/fail result at every stage.
    • The only released-content difference is DMS notifications.yaml (isBanner coerced to a boolean); everything else differs only in formatting.
  • Real Docker builds were run on an arm64 host, plus the containerd image store in a throwaway docker:29-dind.
  • PR Bump got, enforce http2 #466's HTTP/2 fix was checked behind a replica of the production api.ipfs.dappnode.io nginx vhost with Kubo v0.35.0. The old and new code upload to identical CIDs over HTTP/2. With http2: false the original hang reproduces in every run, and the idle timeout fires only on a genuinely stalled upload.
  • Not run: live Pinata, GitHub release and mainnet publish flows.

3alpha added 11 commits November 4, 2026 22:19
Upgrade maintained packages (got 14, octokit 22, ethers 6.17, image-size 2, inquirer 12, form-data 4, tar-fs 3, dotenv 16) and drop unused or deprecated ones (@dappnode/toolkit, dockerode, graphql, request, rimraf), clearing the known lockfile advisories.

- Replace the toolkit APM client with a minimal local ApmRepository.
- ESLint 10 flat config with typescript-eslint 8; lint no longer rewrites files.
- Mocha 12 with tsx; live Docker/IPFS/Ethereum tests move to test:integration and run in a separate, non-blocking CI job.
- Frozen installs in CI and a Node 20/22/24/26 test matrix.
- Narrow js-yaml 4's unknown parse results in the compose, manifest and core-manifest readers.
- Stream IPFS uploads from URLs through got instead of request.
- Shared test fixtures.
Accept human-readable durations and bare integer strings as seconds (the documented --timeout 5000), pass numeric milliseconds through with 0 meaning no timeout, and reject invalid values instead of returning NaN.
Argument arrays now go through execFile, so upstream tags, branch names and image tags are passed literally instead of being interpolated into shell source. Commands accept a cwd, and streamed output is delivered as complete lines. The upstream tag allowlist keeps "@" (e.g. n8n@2.46.6).
A single renderer owns the terminal and shows numbered package/architecture stages with the last complete log lines. --skip_save no longer reports a successful build as an error, --all_variants prints the resolved variant names, and the build and publish handlers forward delete_old_pins.
Reject failed or signalled docker save/xz processes, bound export time and replace the archive atomically, so a broken export is never cached or uploaded. The compose path now pulls external images for the requested architecture, and the architecture check inspects that platform (--platform, with a fallback for Docker CLI < 28.1).
Wait for the overwrite prompt before writing anything, keep an existing Dockerfile unless --force is given, and derive the default package name from --dir.
Release files are now assembled from the same merged root and variant configuration that passed validation.

- Setup wizards are read from JSON or YAML, merged by field id, validated against the schema and emitted as JSON.
- Notifications are merged by correlationId. Within a matched wizard field or endpoint, a variant array replaces the root array instead of being merged by index.
- Single release files (avatar, disclaimer, Prometheus targets, ...) come from the variant directory when present, otherwise from the root. Grafana dashboards are collected from both.
- Values coerced by schema validation are listed in the "Validate files" step so source files can be fixed.
- A single YAML manifest no longer hits the JSON-only template restriction.
- Publishing keeps external upstream image references and recomputes the release directory for the new version.
- Compose validation errors keep the "Invalid compose:" prefix.
- The README documents the root and variant file rules.
- Old pins are deleted only after the replacement upload succeeded, and only for commits that are ancestors of the current commit.
- On shallow CI checkouts the missing history is fetched once before checking. Commits that are still unknown are never unpinned.
- Branch cleanup paginates Pinata pins, rechecks each allegedly deleted branch before unpinning, and handles template packages per variant.
Uploads report when all bytes are sent and then show the time spent waiting for pinning confirmation. Requests now abort only after 15 minutes without socket activity instead of at a fixed total, so slow but healthy uploads of large releases are not cut off. HTTP/2 is kept for IPFS uploads. The Swarm upload URL is relative to prefixUrl.
- GitHub releases upload every defined optional asset (setup wizard, schemas, UI, dashboards, ...).
- Release preparation and upload errors fail the publish. An asset that a failed attempt already stored completely is accepted, and a partial one is replaced.
- Branches and releases are paginated, and tag lookups match the exact ref.
- GitHub action and bump-upstream builds pass --dir, the variants directory and the compose file name to the build, and read git metadata from the package directory.
A failed build printed its command output three or four times, and verbose logs repeated each line up to four times.

- Shell errors carry a summary line plus the last 20 output lines; the full stdout and stderr stay on the error.
- The terminal renderer keeps its own short per-task history, and a failed task shows only its title.
- A verbose renderer logs every streamed line once and leaves the error to the CLI.
- The CLI reports an error once and exits with code 1 instead of rethrowing. It also handles yargs' rejected parse promise, which Node otherwise printed as an unhandled rejection. When the log was already streamed only the summary is shown, and stack frames appear only with --verbose/--debug.
@3alpha
3alpha requested a review from a team as a code owner November 4, 2026 21:42
Comment thread .github/workflows/main.yml Fixed
Comment thread src/utils/shell.ts Fixed
Comment thread src/utils/shell.ts Fixed
@octokit/request depends on content-type 3, which requires Node 22, so frozen installs failed on Node 20 (end of life since April 2026). The dappnode/workflows that run the SDK already use Node 24.

- Engines: >=22.13.0, the strictest requirement across dependencies (ESLint needs ^22.13 || >=24).
- CI matrix 22/24/26. The GitHub-action test workflows and the integration job use Node 24, like production.
- Explicit least-privilege token permissions for main and the GitHub-action test workflows (CodeQL actions/missing-workflow-permissions).
- Shell tests run `node` from PATH instead of process.execPath, which CodeQL traced as an uncontrolled absolute path into execFile.
Every upload attempt opened a new file stream, and a failed attempt never closed it. Besides leaking a file descriptor per retry, a stream still opening after its file was removed raised an unhandled 'error' event, which crashed the test run on Node 24. Each attempt now destroys its stream and handles its error event; read errors still fail the request that consumes the stream.
The manifest repository URL was accepted if it merely contained "https://github.com/" (CodeQL js/incomplete-url-substring-sanitization), and ".git" was removed from anywhere in the name. The URL is now parsed and must be https or git+https on github.com, and only a trailing ".git" is removed. All repository URLs of the 97 dappnode/DAppNodePackage* repos resolve to the same slug as before.
Commenting on a pull request through the issues API needs pull-requests: write; with read the build bot comment failed with 403 "Resource not accessible by integration".
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

Dappnode bot has built and pinned the built packages to an IPFS node, for commit: 7fc8437

This is a development version and should only be installed for testing purposes.

  1. Package dappnodesdk.public.dappnode.eth

Install link

Hash: /ipfs/QmeUTS6qUrriP6fH2rN4nfib1TSkSYgCSiUrTnBeYGxpzQ

(by dappnodebot/build-action)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants