iframe-proxy

Guflly · 2026-07-27T07:45:59Z

Description

Make globally installed commitlint packages resolvable from TypeScript configuration files in the CI image. The image build now exercises a mounted TypeScript fixture that imports both a runtime value and a type from @commitlint/types.

Motivation and Context

The image installs commitlint packages globally, but the TypeScript config loader resolves imports relative to the config file. As a result, configs that import commitlint packages fail with Cannot find module '@commitlint/types'.

Setting NODE_PATH to the image's global module directory makes those imports available to the loader.

Fixes #4623.

Usage examples

import { RuleConfigSeverity } from "@commitlint/types";
import type { UserConfig } from "@commitlint/types";

export default {
	rules: {
		"header-max-length": [RuleConfigSeverity.Error, "always", 100],
	},
} satisfies UserConfig;

How Has This Been Tested?

Reproduced the module resolution failure with globally installed commitlint packages and the TypeScript fixture
Verified the same fixture passes when the global module directory is provided through NODE_PATH
Ran pnpm lint
Ran pnpm build
Ran the focused TypeScript config-loader test
Ran the CLI test suite (71 tests)

A container runtime is not installed locally, so the final Docker image build is left to CI.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist

My change requires a change to the documentation.
I have updated the documentation accordingly.
I have verified that any documentation examples I added/changed actually work.
I have added tests to cover my changes.
All new and existing tests passed.
For a feature/bug fix, my commits follow the test-driven flow.

Copilot

Pull request overview

This PR fixes module resolution for TypeScript-based commitlint configuration files when using the published Docker image, where commitlint packages are installed globally and the TypeScript loader resolves imports relative to the mounted config file.

Changes:

Set NODE_PATH in the container image so the TypeScript config loader can resolve globally installed @commitlint/* packages.
Add a mounted TypeScript fixture and exercise it during the image build to prevent regressions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

escapedcat · 2026-07-27T08:35:20Z

Any of this feedback from Claude valuable?

Findings, most-significant first

Dockerfile.ci:40 / container-build.yml:9 — executable code enters the release pipeline
through a path CI doesn't watch. The smoke test jiti-executes
.github/fixtures/docker/commitlint.config.ts (arbitrary JS, as root) inside a final-stage RUN
layer of the image that ships to Docker Hub. But container-build.yml's pull_request trigger is
paths: ["Dockerfile.ci"] only — it does not include .github/fixtures/docker/. So a later PR
that edits only the fixture triggers no container build at review time (reviewers see "just a
test fixture," no image-build check runs), yet that code executes unattended at the next
nightly cron or release-tag build. This needs merge access to exploit, so it isn't external
RCE — but it lowers review visibility for code that runs in the publish pipeline, and it
compounds the existing "publish only fails at tag time" fragility. Fix: inline the config into
Dockerfile.ci via a heredoc (keeps every executed byte under the paths filter and drops the
separate file), or move the smoke test to a non-shipping build stage, or at minimum add
.github/fixtures/docker/ to the workflow's paths list.

Dockerfile.ci:33 — ENV NODE_PATH changes module resolution for every end-user of the image,
not just the build. The ENV persists into the shipped image (and images derived from it), so
any commitlint run in the container now falls back to resolving bare import/extends from the
image's entire global tree — all ~19 @commitlint/* packages, their transitive deps, and
npm/corepack internals. A user whose mounted config imports/extends a package they forgot to
declare will now silently resolve the image's pinned copy and pass, while the same repo fails
or behaves differently on their local non-Docker run — "works only in Docker," with version
skew invisible. It's the intended fix mechanism, but scoping resolution to the smoke-test RUN
(rather than a stage-wide ENV) would preserve fail-fast behavior for real users; if the broad
behavior is intended, it deserves a doc line.

Dockerfile.ci:40 — the smoke test is positive-only and can pass vacuously. commitlint exits
0 both when the rule is applied and when the config silently loads as empty {}, and the
30-char test message satisfies header-max-length: 100 regardless. A future jiti/loader
regression that returns {} instead of throwing keeps the build green while TS-config support
is actually broken. Pipe a >100-char header and assert exit 1 to make the test exercise the
rule.

Dockerfile.ci:40 — build residue ships in the image. Confirmed in the built image: the
smoke-test RUN leaves /tmp/jiti/tmp-commitlint.config.*.mjs (jiti's transpile cache) and a
/tmp/node-compile-cache/ tree in the published layer. Minor bloat, and /tmp/jiti is root-owned
so a --user-non-root run could collide on it. rm -rf /tmp/jiti /tmp/node-compile-cache in the
same RUN (or a non-shipping stage) clears it.

.github/fixtures/docker/commitlint.config.ts:1 — fix depth is loader-specific; the fixture
doesn't cover the issue's actual repro. Issue fix: docker image throws error for TypeScript config files #4623's config also has extends:
['@commitlint/config-conventional']; the fixture drops it. extends resolves via a different
mechanism (global-directory, execPath-based, independent of NODE_PATH) so it already worked —
but the test never proves the reported config passes. Separately, an ESM config (.mjs /
type:module) with the same import stays broken, because native ESM ignores NODE_PATH — and the
working CJS path leans on jiti's last-resort native-require fallback, so a jiti bump or
Node's native TS type-stripping could silently re-break it. A resolution fallback in
@commitlint/load would fix the Docker image, bare npm i -g installs, and ESM configs in one
place. Adding the extends line to the fixture at least upgrades the test from tailored-to-pass
to covering the issue.

container-build.yml — this PR can't be validated by its own CI, and adds a release-time
flake point. It's a fork PR; on pull_request from a fork GitHub withholds DOCKERHUB_* secrets,
so docker/login-action fails before the build step and the new smoke test never runs
pre-merge (gh pr checks shows no checks). Its first real execution is a post-merge nightly or
release-tag build — where it now also runs node+jiti under QEMU arm64 emulation, stacking a
flake/duration risk onto the npm-CDN race this Dockerfile already hits at tag time. Worth a
local docker build before merging (I effectively did this — it passes).

Recommendation: technically sound and safe to consider, but I'd ask for two changes before
merge — inline the smoke-test config into Dockerfile.ci (resolving finding 1 and eliminating
the fixture file) and clean up the cache residue (finding 4) — and I'd raise scoping NODE_PATH
to the RUN vs. stage-wide (finding 2). Findings 3, 5, and 6 are reasonable follow-up asks
depending on how much you want the test to actually guarantee.

Guflly · 2026-07-27T20:00:43Z

Yes. I inlined the config, added a failing long-header check, included the extends case from the issue, and clean up the loader caches. I kept NODE_PATH on the image because limiting it to the build check would leave mounted TypeScript configs broken at runtime.

qodo-code-review · 2026-07-27T20:01:50Z

PR Summary by Qodo

fix(container): resolve global commitlint packages from TypeScript configs

🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 10-20 Minutes

AI Description

• Set NODE_PATH so TypeScript config imports can resolve globally installed commitlint packages.
• Add an image build smoke test that loads a TS config importing @commitlint/types.
• Verify the TS config rule executes by asserting an expected commitlint failure.

Diagram

graph TD
A["Dockerfile.ci"] --> B["npm -g install commitlint"] --> C["ENV NODE_PATH"] --> D["Write TS config fixture"] --> E["Run commitlint with --config"]
E --> F["Global node_modules"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Install commitlint deps locally (non-global) in the image

➕ Avoids relying on NODE_PATH (legacy/implicit resolution)
➕ Module resolution matches typical Node project behavior
➖ More image complexity (workspace directory + node_modules)
➖ Potentially larger image and slower build/install

2. Symlink global modules into a known project node_modules directory

➕ Keeps global install but makes resolution explicit via standard node_modules lookup
➖ Extra filesystem wiring and potential fragility across base images/npm versions
➖ Still indirect vs making loader aware of global install path

3. Change config-loader invocation to inject resolver paths (if supported)

➕ Avoids global NODE_PATH side effects
➕ Resolution behavior scoped to the loader/command
➖ May not be supported by commitlint’s loader stack (jiti/ts-node)
➖ More code/CLI surface area to maintain

Recommendation: The chosen approach (setting NODE_PATH in the CI image) is the most direct and least invasive way to make globally installed commitlint packages resolvable by the TypeScript config loader. The added build-time fixture meaningfully reduces regression risk. Consider documenting NODE_PATH’s intent in the Dockerfile since it affects Node resolution globally within the container.

Files changed (1) +20 / -0

Bug fix (1) +20 / -0

Dockerfile.ciSet NODE_PATH and add TS-config smoke test during image build +20/-0

Set NODE_PATH and add TS-config smoke test during image build

• Adds NODE_PATH=/usr/local/lib/node_modules so TypeScript commitlint configs can import globally installed @commitlint/* packages. Introduces a build-time smoke test that writes a temporary commitlint.config.ts importing both a runtime enum and a type from @commitlint/types, runs commitlint to confirm loading, and asserts the custom rule rejects an overlong header before cleaning up temporary caches/files.

Dockerfile.ci

greptile-apps · 2026-07-27T20:03:32Z

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
Dockerfile.ci	Configures global module resolution and adds a build-time TypeScript configuration smoke test; no eligible follow-up issue was identified.

_{Reviews (3): Last reviewed commit: "test(container): prove custom TypeScript..." | Re-trigger Greptile}

qodo-code-review · 2026-07-27T20:03:49Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. BuildKit-only heredoc 🐞 Bug ☼ Reliability

Description

Dockerfile.ci now uses a RUN <<'EOF' heredoc, which fails to build under the legacy Docker builder
(e.g., with DOCKER_BUILDKIT=0). This introduces a build portability regression for anyone building
this image without BuildKit enabled.

Code

Dockerfile.ci[40]
+RUN <<'EOF'

Evidence

The Dockerfile introduces a heredoc-based RUN instruction, which is a BuildKit-only Dockerfile
frontend feature. The project’s container build workflow uses Docker Buildx (BuildKit), indicating
CI support, but the Dockerfile itself would not be parseable by the legacy builder.

Dockerfile.ci[40-58]
.github/workflows/container-build.yml[18-45]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Dockerfile.ci` uses `RUN <<'EOF'` heredoc syntax. This requires the BuildKit Dockerfile frontend; legacy docker builds (BuildKit disabled) will fail to parse the Dockerfile.

## Issue Context
The repo’s GitHub workflow uses Buildx (BuildKit), so CI is fine, but local/manual or downstream builds using the classic builder can break.

## Fix Focus Areas
Choose one:
1) **Make Dockerfile compatible without heredocs**: Replace the heredoc with a classic `RUN /bin/sh -c '...'` command (using `printf`/`cat <<"CONFIG"` within the single-quoted string) or `COPY` a small script into the image and `RUN` it.
2) **Explicitly require BuildKit**: Add a short comment near the syntax usage (or in contributor docs) stating that building `Dockerfile.ci` requires BuildKit/Buildx.

### Code references
- Dockerfile.ci[40-58]
- .github/workflows/container-build.yml[18-45]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Guflly · 2026-07-28T02:43:37Z

The workflow runs are waiting for maintainer approval before the jobs can start.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

escapedcat · 2026-08-02T19:29:04Z

Does this comment from Claude make sense?

The negative test uses header-max-length: 100, which is exactly what @commitlint/config-conventional already sets — so this check would still pass even if the custom rules block from the TS config were silently ignored. To make it prove the TS rules are actually applied, use a value that differs from the preset, e.g. [RuleConfigSeverity.Error, "always", 50] and a ~60-char test header.

Guflly · 2026-08-03T08:09:47Z

Yes, that makes sense. I changed the custom limit to 50 and use a 61-character header, so the check now proves the TypeScript rules block is being applied.

escapedcat · 2026-08-03T16:48:03Z

escapedcat requested a review from Copilot July 27, 2026 07:57

Copilot started reviewing on behalf of escapedcat July 27, 2026 07:57

View session

Copilot AI reviewed Jul 27, 2026

View reviewed changes

Guflly marked this pull request as ready for review July 27, 2026 20:00

qodo-code-review Bot reviewed Jul 27, 2026

View reviewed changes

Comment thread Dockerfile.ci

Guflly added 3 commits August 2, 2026 11:45

test(container): cover TypeScript config imports

1853d14

fix(container): resolve globally installed packages

c4bc2d6

test(container): strengthen TypeScript config check

6b1b937

Guflly force-pushed the fix/docker-typescript-config branch from 8ff5f04 to 6b1b937 Compare August 2, 2026 18:45

escapedcat requested a review from Copilot August 2, 2026 19:17

Copilot started reviewing on behalf of escapedcat August 2, 2026 19:17

View session

Copilot AI reviewed Aug 2, 2026

View reviewed changes

test(container): prove custom TypeScript rules are loaded

1a1c0c0

escapedcat merged commit f5712a9 into conventional-changelog:master Aug 3, 2026
12 of 13 checks passed

File	Description
Dockerfile.ci	Sets `NODE_PATH` and validates TypeScript config loading during the image build.
.github/fixtures/docker/commitlint.config.ts	Adds a minimal TypeScript config that imports runtime and type exports from `@commitlint/types`.

Sunbelt Computer Software

PL/B Language Development and Support

Conversation

Guflly commented Jul 27, 2026

Description

Motivation and Context

Usage examples

How Has This Been Tested?

Types of changes

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

escapedcat commented Jul 27, 2026

Uh oh!

Guflly commented Jul 27, 2026

Uh oh!

qodo-code-review Bot commented Jul 27, 2026

PR Summary by Qodo

Uh oh!

greptile-apps Bot commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 5/5

Important Files Changed

Uh oh!

qodo-code-review Bot commented Jul 27, 2026

Code Review by Qodo

Uh oh!

Uh oh!

Guflly commented Jul 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

escapedcat commented Aug 2, 2026

Uh oh!

Guflly commented Aug 3, 2026

Uh oh!

Uh oh!

escapedcat commented Aug 3, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

greptile-apps Bot commented Jul 27, 2026 •

edited

Loading