ci: fix max-lines ratchet + knip on bundle-all-plugins (follow-up to #182) - #186
vashbrightfire[bot] wants to merge 2 commits into
Conversation
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.
a4a5e07 to
258a053
Compare
💡 Codex Reviewhttps://github.com/brightfire/openclaw/blob/a4a5e073a38b62371f16bc68d0d35bc9a4c6b9a6/test/scripts/release-plan-producer.test.ts#L1241 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 AGENTS.md reference: AGENTS.md:L281-L281 ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
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):
checks-fast-baseline-ratchets:src/plugins/bundled-plugin-metadata.test.ts(1142 raw lines) carried a grandfatheredoxlint-disable max-linesthat was never inconfig/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).check-dependencies(knip):get-east-asian-widthis flagged as an unused root dependency. It is added by the bundle-all-plugins patch and consumed only byshouldAlwaysBundleDependency()intsdown.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.tskeeps 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 incheck-changed/test-projectsstay valid unchanged.src/plugins/bundled-plugin-activation.test.tstakes the startup/CLI activation declarations and Gateway startup resolution tests (205 effective lines). It routes to theunit-fast-isolatedvitest project via the repo's stateful-test-support classifier — the standard routing for test-support consumers.src/plugins/bundled-plugin-metadata.test-support.tsholds the shared repo-level fixtures and expectation helpers (192 effective lines), following the existingsrc/plugins/*-test-support.tsconvention (knip's**/*test-support.tsignores 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. Addedget-east-asian-widthto the root workspaceignoreDependenciesinconfig/knip.config.tswith a comment naming the consumer (tsdown.config.ts shouldAlwaysBundleDependency(), bundle-all-plugins), following the established pattern of the existinghighlight.jscreateRequire 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 through19ee23d8ff2) 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 withgit 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-fixes5cb15f9a0958+ slack-mrkdwn + cli-http-fallback2966a901bec+ webhook-sessiontarget-support + otel-improvements + bundle-all-plugins43c8b3b5459+ this branch), plus the exact branch-head tree:pnpm teston both files via repo project routing: 37/37 green (30 inplugins, 7 inunit-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 --checkon all four touched files: clean; targeted oxlint on them: 0 warnings / 0 errorsknip@6.32.2 --config config/knip.config.ts --production --no-progress --reporter compact --no-config-hints --exclude duplicates): exit 0, no unused-dependency finding forget-east-asian-widthdeadcode:unused-filesat the branch-head tree: exit 0 / passtest-projects.test.ts,changed-lanes.test.ts,vitest-unit-paths.test.ts): 581 passed / 1 skippedPre-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: exerciseTerminalOutputSafetyas unused. It does not affect per-PR CI; it will surface in the next full stack build.