Skip to content

ci: fix max-lines ratchet + knip on bundle-all-plugins (follow-up to #182) - #186

Closed
vashbrightfire[bot] wants to merge 2 commits into
brightfire/999239d745d/bundle-all-pluginsfrom
skip-bundle-policy-tests
Closed

vashbrightfire[bot] wants to merge 2 commits into
brightfire/999239d745d/bundle-all-pluginsfrom
skip-bundle-policy-tests

Conversation

@vashbrightfire

@vashbrightfire vashbrightfire Bot commented Sep 8, 2026

Copy link
Copy Markdown

What Problem This Solves

Two CI checks fail on the bundle-all-plugins patch tree for pre-existing patch hygiene (both surfaced when the patch stack got PR CI for the first time via #182; neither is a regression from that PR's test rework):

  1. checks-fast-baseline-ratchets: src/plugins/bundled-plugin-metadata.test.ts (1142 raw lines) carried a grandfathered oxlint-disable max-lines that was never in config/max-lines-baseline.txt, so the ratchet flags it as a new suppression. The test-file budget is 1000 effective lines (skipBlankLines/skipComments; measured 1046).
  2. check-dependencies (knip): get-east-asian-width is flagged as an unused root dependency. It is added by the bundle-all-plugins patch and consumed only by shouldAlwaysBundleDependency() in tsdown.config.ts, which knip cannot see because the consumption is a string-based bundler policy.

Why This Change Was Made

ebca7ca5056 — split the test file. Split into three files, each under budget:

  • src/plugins/bundled-plugin-metadata.test.ts keeps the repo metadata, runtime sidecar baseline, public-surface, and generated-path resolution contracts (672 effective lines). It keeps the original filename so the runtime-sidecar-baseline owner mappings in check-changed/test-projects stay valid unchanged.
  • src/plugins/bundled-plugin-activation.test.ts takes the startup/CLI activation declarations and Gateway startup resolution tests (205 effective lines). It routes to the unit-fast-isolated vitest project via the repo's stateful-test-support classifier — the standard routing for test-support consumers.
  • src/plugins/bundled-plugin-metadata.test-support.ts holds the shared repo-level fixtures and expectation helpers (192 effective lines), following the existing src/plugins/*-test-support.ts convention (knip's **/*test-support.ts ignores apply; every export is consumed by the two test files).

All 37 test cases are preserved byte-for-byte (30 + 7); zero skips existed before and after. The max-lines suppression is deleted; no baseline entry added.

258a0535a2 — knip ignore. Added get-east-asian-width to the root workspace ignoreDependencies in config/knip.config.ts with a comment naming the consumer (tsdown.config.ts shouldAlwaysBundleDependency(), bundle-all-plugins), following the established pattern of the existing highlight.js createRequire entry.

Branch contents note: this branch was originally created on the pre-merge #182 head and rebased onto 51e69dc968d9 (the #182 squash merge, content-equal through 19ee23d8ff2) after that PR merged, so the branch is a strict descendant of the base and the PR diff shows only the split + knip changes — verified with git diff origin/brightfire/999239d745d/bundle-all-plugins..skip-bundle-policy-tests --stat: 4 files, +449/−420.

User Impact

None — test files and the knip config only. No product code changes.

Evidence

Full six-patch stack rebuilt fresh from 999239d745d (upstream-test-fixes 5cb15f9a0958 + slack-mrkdwn + cli-http-fallback 2966a901bec + webhook-sessiontarget-support + otel-improvements + bundle-all-plugins 43c8b3b5459 + this branch), plus the exact branch-head tree:

  • pnpm test on both files via repo project routing: 37/37 green (30 in plugins, 7 in unit-fast-isolated); test count unchanged from the pre-split file (37 cases, 0 skips)
  • pnpm check:max-lines-ratchet: OK (874 grandfathered suppressions; this file's suppression removed, none added)
  • oxfmt --check on all four touched files: clean; targeted oxlint on them: 0 warnings / 0 errors
  • knip production scan with the exact CI command (knip@6.32.2 --config config/knip.config.ts --production --no-progress --reporter compact --no-config-hints --exclude duplicates): exit 0, no unused-dependency finding for get-east-asian-width
  • knip all-exports scan + deadcode:unused-files at the branch-head tree: exit 0 / pass
  • tsgo core-test shards: pass
  • Classification guard suites (test-projects.test.ts, changed-lanes.test.ts, vitest-unit-paths.test.ts): 581 passed / 1 skipped

Pre-existing stack-level note (present on the six-patch stack without this branch): the new upstream-test-fixes head combined with the other patches makes the all-exports scan flag src/tui/tui-pty-harness-assertion-test-support.ts: exerciseTerminalOutputSafety as unused. It does not affect per-PR CI; it will surface in the next full stack build.

vashbrightfire[bot] added 2 commits September 8, 2026 12:35
Remove the grandfathered max-lines suppression by splitting the 1142-line
test file into two files under the 1000-line test budget:

- bundled-plugin-metadata.test.ts keeps the repo metadata, sidecar
  baseline, public-surface, and generated-path resolution contracts.
- bundled-plugin-activation.test.ts takes the startup/CLI activation
  declarations and Gateway startup resolution tests.
- bundled-plugin-metadata.test-support.ts holds the shared repo-level
  fixtures and expectation helpers both files consume.

All 37 test cases are preserved byte-for-byte; no skips existed on this
head. The runtime sidecar baseline owner tests stay in the original
filename so the check-changed/test-projects mappings remain valid.
The dependency is consumed only by tsdown.config.ts's
shouldAlwaysBundleDependency() (bundle-all-plugins), which knip cannot
see because the consumption is a string-based bundler policy, so the
root workspace scan flags it as unused. Ignore it with a comment naming
the consumer.
@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

https://github.com/brightfire/openclaw/blob/a4a5e073a38b62371f16bc68d0d35bc9a4c6b9a6/test/scripts/release-plan-producer.test.ts#L1241
P1 Badge Keep WhatsApp in the plugin release inventory

When the next release plan is generated, this zero-ClawHub expectation—together with the adjacent four-package npm expectation—accepts omitting every plugin package, including @openclaw/whatsapp. WhatsApp is explicitly excluded from the core build in scripts/lib/bundled-plugin-build-entries.mjs:21, while scripts/lib/plugin-publication-collector.ts:83-88 suppresses publication for its new bundledDist: true state; extensions/whatsapp/package.json:65-80 and docs/channels/whatsapp.md:10-18 still require the npm/ClawHub install route. A release would therefore contain no bundled WhatsApp runtime and no updated external package, so preserve WhatsApp as an externally published exception rather than ratcheting the inventory to zero.

AGENTS.md reference: AGENTS.md:L281-L281

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

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