Skip to content

port(upstream): #2474 - Add Node.js CI workflow#141

Closed
bloodf wants to merge 1 commit into
devfrom
port/upstream-2474-a3c97f2
Closed

port(upstream): #2474 - Add Node.js CI workflow#141
bloodf wants to merge 1 commit into
devfrom
port/upstream-2474-a3c97f2

Conversation

@bloodf

@bloodf bloodf commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Source: decolua/9router#2474
Lock: origin/dev@a3c97f2a49910278081dfcf1c9d6f847d2da6d6d

Scope

Hand-ports the upstream Add Node.js CI workflow file (.github/workflows/node.js.yml, +31/-0 upstream) onto the DurinDoor two-branch release model.

Fork-only touchpoints / adaptations

The upstream file targets a single master branch, so the workflow was adapted rather than copied verbatim:

  • Triggers master -> dev + main (matches ci.yml, test.yml, nightly.yml).
  • npm ci + cache: npm kept: package-lock.json is tracked in this fork (verified with git ls-files), so the upstream clean-install/cache pattern is valid here. This also ensures a PR that changes package.json without updating the lockfile is caught.
  • npm run build --if-present -> unconditional npm run build: this fork always defines a build script (Next.js --webpack); --if-present would silently no-op on a regression and hide it.
  • npm test omitted: DurinDoor already runs the vitest suite and no-regression gate in test.yml. Including it here would duplicate that job and fail on the same inherited baseline drifts. This workflow is therefore a focused install-and-build smoke check.
  • Node matrix [18.x, 20.x, 22.x] -> single 20: matches ci.yml / test.yml and bounds Actions minutes.
  • Added concurrency group + permissions: contents: read to match the fork's other workflows.

Review response

  • chatgpt-codex-connector: fixed in 9307ebad6 — switched from npm install to npm ci and restored cache: npm now that the lockfile is confirmed tracked.

Test coverage

No new test files — this is a CI-only change (AGENTS §1 docs-only exemption). The workflow exercises the existing build, so coverage is inherited from tests/ and ci.yml.

Doc coverage

CI-only change; this PR body is the documentation. No user-facing behavior change, so no CHANGELOG.md entry.

Baseline impact

None — does not touch tests/__baseline__/known-fails.txt.

Wire-format / migration concerns

None.

Note on commit subject

Task spec asked for port(upstream/#2474): ...; the conventional-commit parser rejects / inside a scope, so the commit/PR subject uses the AGENTS §3 form port(upstream): #2474 - <title> instead (validated locally with npx commitlint).

@bloodf
bloodf force-pushed the port/upstream-2474-a3c97f2 branch from 48138e8 to 9789bfa Compare July 9, 2026 15:04

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9789bfa49b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/node.js.yml Outdated
Source: decolua/9router#2474

Lock: origin/dev@a3c97f2a49910278081dfcf1c9d6f847d2da6d6d

Hand-ported the upstream 'Add Node.js CI workflow' file onto the two-branch DurinDoor release model. Upstream triggers on master; this fork targets dev + main. Keeps npm ci and cache:npm because package-lock.json is tracked in this fork (verified via git ls-files). Build made unconditional to avoid silently hiding regressions, and Node matrix narrowed to 20 to match ci.yml / test.yml. Upstream npm test step omitted because DurinDoor already runs the vitest suite and no-regression gate in test.yml.

Fixes chatgpt-codex-connector review: use the committed lockfile (npm ci + cache:npm).
@bloodf
bloodf force-pushed the port/upstream-2474-a3c97f2 branch from 9789bfa to 9307eba Compare July 9, 2026 15:10
@bloodf

bloodf commented Jul 9, 2026

Copy link
Copy Markdown
Owner Author

@chatgpt-codex-connector Fixed the lockfile concern in : restored
added 601 packages in 28s and now that is confirmed tracked. I attempted to resolve the review thread via the API, but the mutation is unavailable in this token scope; please mark the thread resolved if the fix looks correct.

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

@bloodf

bloodf commented Jul 9, 2026

Copy link
Copy Markdown
Owner Author

chatgpt-codex-connector: Fixed the lockfile concern in commit 9307eba. Restored npm ci and cache:npm now that package-lock.json is confirmed tracked. The resolvePullRequestReviewThread mutation is not available via this token scope, so I cannot mark the thread resolved; please resolve it if the fix looks correct.

@bloodf

bloodf commented Jul 10, 2026

Copy link
Copy Markdown
Owner Author

Closing this redundant workflow unmerged. Source #2474 remains open and unchanged at 80ee0f89bfa629e0c08a4b01276aa05eac4b0087. Its valid lockfile concern is fully retained by canonical recovery #145/#150: exact Node 20.20.2 and npm 10.8.2, root/tests lockfiles, npm ci, fail-closed tests, verified artifacts, and the three required ruleset checks. The single review thread is resolved; a third overlapping Node CI workflow would only duplicate and confuse check ownership.

@bloodf bloodf closed this Jul 10, 2026
@bloodf
bloodf deleted the port/upstream-2474-a3c97f2 branch July 17, 2026 02:50
@bloodf bloodf mentioned this pull request Jul 20, 2026
8 tasks
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.

1 participant