Skip to content

Follow-ups from PR #1034 (hygiene rollout): retire validate-conventional-commits.js, fix comfyui typecheck, tighten job permissions #1035

@willgriffin

Description

@willgriffin

PR #1034 (chore(config): hygiene rollout) deferred two items as out-of-scope for the hygiene PR. Capturing here so they don't get lost.

1. Retire scripts/validate-conventional-commits.js

Background: PR #1034 originally had two parallel commit-message validators:

  • The lefthook commit-msg hook + wagoid/commitlint-github-action in CI, both consuming commitlint.config.mjs
  • scripts/validate-conventional-commits.js, a hand-rolled regex validator with a permissive scope grammar and 50-char subject limit — invoked from .github/workflows/on-pull-request.yml for PR-title validation and from scripts/release-helper.js:194

PR #1034's round 4 fix replaced the workflow's invocation with pnpm exec commitlint, so the two gates now share one source of truth. But scripts/release-helper.js still calls the standalone validator. This means:

  • A commit message that passes scripts/release-helper.js's preflight can still fail commitlint.config.mjs (different rules: permissive scope, 50-char subject).
  • The standalone validator's grammar is documented separately from commitlint.config.mjs — easy to drift further.

Acceptance criteria:

  • scripts/release-helper.js validates commit messages via commitlint (using commitlint.config.mjs) instead of the hand-rolled regex.
  • scripts/validate-conventional-commits.js deleted (no remaining consumers).
  • scripts/release-helper.js tests still pass; CI green.
  • commitlint.config.mjs docblock updated to drop the "still calls scripts/validate-conventional-commits.js" caveat (currently lines 12-17).

Out of scope here: any broader refactor of scripts/release-helper.js beyond the validator swap.


2. Fix pre-existing TypeScript error packages/comfyui/src/client.ts:269

Background: PR #1034's Validate Changes / Run Tests CI check failed on commit 1ed2ff4b:

src/client.ts:269:32 - error TS2339: Property 'exception_message' does not exist on type '{}'.

269           error: message.data?.exception_message,
                                   ~~~~~~~~~~~~~~~~~

The file is in packages/comfyui and is not touched by PR #1034. The error is pre-existing in main — verified via git diff origin/main..HEAD -- "**/client.ts" returning empty. Main's CI was already red on this before #1034 opened.

What the code does: message.data?.exception_message is reading from a WebSocket message payload. message.data is typed {} (empty object), so TypeScript can't see any properties on it. The runtime data has the field; the type is just stale.

Acceptance criteria:

  • pnpm typecheck passes in packages/comfyui (and in the full workspace).
  • message.data is given a proper type (interface for the WebSocket payload shape) rather than {} — or, if intentional any-style usage, cast explicitly + comment why.
  • Regression test: any other call sites accessing message.data?.<field> are checked for the same pattern and fixed in one pass.

Out of scope here: the broader 17 dependabot vulnerabilities flagged on git push (2 critical, 10 high, 5 moderate) — those deserve their own triage.


3. (Optional, low-priority) Tighten validate-commits job permissions

Background: The validate-commits job in .github/workflows/on-pull-request.yml inherits workflow-level permissions which include pull-requests: write and packages: read. The commit-validation step only needs contents: read. Per the audit that informed PR #1034, this could be tightened with per-job permissions: blocks.

Acceptance criteria:

  • validate-commits job has its own permissions: { contents: read } block.
  • Other jobs in the workflow audited for similar over-broad inheritance.

Why deferred: tightening permissions deserves a careful per-job audit, not a one-line drive-by. PR #1034's scope was already exceeded.


Cross-ref: PR #1034 (sdk-hygiene), round 5 + round 4 commit messages.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions