chore: turn on the checks that were, well, not on - #1106
Open
teallarson wants to merge 23 commits into
Open
Conversation
Deletion and documentation accuracy only; no behavior change. - CLAUDE.md documented `pnpm build` as a three-stage pipeline ending in pagefind. Pagefind does not exist in this repo (search is an external Algolia crawler); the build is a single `next build --webpack`. Adds a Vale install note, since `pnpm vale:check` is documented as required but vale is a Go binary with no npm dependency. - .gitignore reserved `public/toolkit-markdown/` for a build step that no longer exists, and both .gitignore and the Makefile referenced `make_toolkit_docs/`, a Python directory that was removed. `make mcp-server-docs` was therefore a broken target. - `data/toolkits/jira.json` was an unreferenced 133 KB copy at the repo root; the live data is under `toolkit-docs-generator/data/toolkits/`. - Drops unused dependencies (zustand, turndown, @mdx-js/react) and redundant direct declarations that are supplied transitively (@theguild/remark-mermaid via nextra, baseline-browser-mapping via next, unist-util-visit-parents, mdast-util-to-string). Moves chalk to devDependencies and consolidates the two colour libraries onto it. - Adds @types/hast so neutralize-emails.tsx can use unist-util-visit instead of a hand-rolled tree walk. - The nightly generator workflow ran `pnpm build` with a working-directory that has no package.json, so pnpm resolved upward and executed the root Next production build. The step that follows runs the CLI through tsx and needs no build. - Renames ignored-toolkits.txt/excluded-toolkits.txt to skip-toolkits.txt/remove-toolkits.txt, which say what they do. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two isolated behavior fixes. Static rendering: the root layout awaited headers() to read "x-pathname" and derive a locale. Awaiting headers() in the root layout opts the entire route tree out of static rendering, so every page — including all 117 toolkit pages, which are pure functions of committed JSON — was server-rendered on demand. The derived locale was always "en": proxy.ts redirects every non-English locale to /en and getPreferredLocale returns "en" unconditionally. The site paid full dynamic rendering to compute a constant. This is not an i18n change. TranslationBanner and the dictionary plumbing stay in place; restoring real i18n means an app/[lang]/ route segment, which is the correct Next pattern regardless. Sitemap: app/sitemap.ts skips any directory whose name contains "[", which is right for directory walking but meant all 117 toolkit pages were absent from sitemap.xml — the largest content section on the site. Merges in listValidIntegrationLinks() from app/_lib/toolkit-static-params.ts, the same enumeration the integrations index uses, and dedupes against the authored partner pages the disk walk already finds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The header comment still listed "Build the toolkit docs generator" as step 1 after that step was removed. Renumber the remaining three. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Eliminates the duplication that lets these subsystems drift.
Redirects were a 909-line array inside next.config.ts, so three consumers
regex-parsed the config as text — one of them carrying a second "reversed"
regex that existed only because a human might write {destination, source}
instead of {source, destination}, a problem that only exists when you parse
text instead of importing data. The array now lives in a typed redirects.ts
and every consumer imports it. Two further consumers that also parsed the
config as text (scripts/update-internal-links.ts and
tests/integration-index-links.test.ts) would have silently found zero
redirects, so they move to the import too. --auto-fix now appends to the
data file, and its six scattered "Auto-added redirects" marker blocks
collapse to one append point. The resolved array is byte-identical: 156
entries, same order.
check-redirects-utils.ts had 425 lines of tests but was imported by nothing
except its own test file, while the code that actually runs — the
pre-commit hook's scripts/check-redirects.ts — kept private copies of the
same six helpers. The tests guarded a copy while the shipping
implementation was untested. The module moves to scripts/lib/ and the
shipping script now imports it.
Toolkit primitives (data dir, toKebabCase, normalizeToolkitId, the category
list, the *Api heuristic, docsLink→slug) existed in 2-7 copies, one pair
carrying a "must stay in sync" comment. They collapse into
toolkit-docs-generator/src/shared/, which both halves can import. All seven
data-dir consumers now honor TOOLKIT_DATA_DIR; previously only two did.
The Node-only path resolution lives in its own module because client
components reach the primitives through the integrations index, and a
node:* import anywhere in that graph fails the webpack browser build.
The consumer-side contract was a four-field duck-check that never verified
tools was an array, followed by an unchecked cast — while toToolkitSummary
immediately calls data.tools.map(). The generator's Zod schemas are now the
single shared contract, the 522-line hand-written mirror is z.infer, and
zod moves to dependencies because it enters the app's runtime path.
Corruption is now loud and absence stays quiet: three catch blocks treated
missing, unparseable, and schema-invalid identically, so a malformed file
from the nightly PR silently dropped a toolkit and 404'd. Unparseable or
invalid now throws with the file path and the Zod issues, failing the
build. An unrecognized category throws instead of being coerced to
"others", which had no route directory and would have made every toolkit
in a new category a clickable card pointing at a 404.
One cache()-wrapped loader replaces 11 full passes over the data directory
per build, and removes the scan-every-file miss path that a burst of
unknown IDs could otherwise trigger at request time.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a typecheck script covering ~18k lines that nothing checked: root tests/, and the generator's scripts/ and tests/. scripts/ already had a tsconfig and already passed — it was simply wired to no command. The generator's tsconfig excluded **/*.test.ts, which is what hid most of the errors. Fixing the 50 surfaced errors needed no suppressions and no loosened compiler options: six were .ts import specifiers that tsx tolerates and tsc does not, the rest were incomplete test fixtures and genuine exactOptionalPropertyTypes / noUncheckedIndexedAccess findings. test.yaml now runs it. The generator's vitest config set 80% coverage thresholds and enabled typecheck, but nothing pointed at it — the root config's default glob picked those tests up and ran them under root settings. It could not have worked if it had been wired up: @vitest/coverage-v8 is not installed, so the thresholds could never execute, and running under it breaks four suites for want of an @ alias. Deleting it stops the file implying guarantees it never provided. The six app-lib tests move to root tests/, where the @ alias means they no longer reach through ../../../. The pre-commit hook stashed unstaged work and re-formatted staged files after lint-staged had already formatted them — its own comment conceded the redundancy. That block also swallowed formatter failures by assigning FORMAT_EXIT_CODE=0 unconditionally, and called sha256sum, which stock macOS does not have, so under set -e a contributor without coreutils could not commit at all. 222 lines to 125. The generator fabricated metadata when the design system had none: category "development", an iconUrl, and a docsLink under /en/mcp-servers/, signalled only by a console warning inside an automated PR. One committed record runs on it today. Missing metadata now fails under --require-complete and names the toolkit; without the flag the omission reaches the run log rather than only stdout. The fabricated docsLink prefix now matches the route shape the other 116 records use, and the placeholder is marked hidden so a guessed category cannot file a toolkit under the wrong sidebar section. --require-complete was previously unreachable in --all mode, which is the mode CI uses: a pre-filter added those toolkits to the skip list before the merger ever saw them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The scheduled workflow uses both flags together; metadata-missing toolkits were excluded from change detection and the no-op path never validated them. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
@cursor review |
Two isolated behavior fixes. Static rendering: the root layout awaited headers() to read "x-pathname" and derive a locale. Awaiting headers() in the root layout opts the entire route tree out of static rendering, so every page — including all 117 toolkit pages, which are pure functions of committed JSON — was server-rendered on demand. The derived locale was always "en": proxy.ts redirects every non-English locale to /en and getPreferredLocale returns "en" unconditionally. The site paid full dynamic rendering to compute a constant. This is not an i18n change. TranslationBanner and the dictionary plumbing stay in place; restoring real i18n means an app/[lang]/ route segment, which is the correct Next pattern regardless. Sitemap: app/sitemap.ts skips any directory whose name contains "[", which is right for directory walking but meant all 117 toolkit pages were absent from sitemap.xml — the largest content section on the site. Merges in listValidIntegrationLinks() from app/_lib/toolkit-static-params.ts, the same enumeration the integrations index uses, and dedupes against the authored partner pages the disk walk already finds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Eliminates the duplication that lets these subsystems drift.
Redirects were a 909-line array inside next.config.ts, so three consumers
regex-parsed the config as text — one of them carrying a second "reversed"
regex that existed only because a human might write {destination, source}
instead of {source, destination}, a problem that only exists when you parse
text instead of importing data. The array now lives in a typed redirects.ts
and every consumer imports it. Two further consumers that also parsed the
config as text (scripts/update-internal-links.ts and
tests/integration-index-links.test.ts) would have silently found zero
redirects, so they move to the import too. --auto-fix now appends to the
data file, and its six scattered "Auto-added redirects" marker blocks
collapse to one append point. The resolved array is byte-identical: 156
entries, same order.
check-redirects-utils.ts had 425 lines of tests but was imported by nothing
except its own test file, while the code that actually runs — the
pre-commit hook's scripts/check-redirects.ts — kept private copies of the
same six helpers. The tests guarded a copy while the shipping
implementation was untested. The module moves to scripts/lib/ and the
shipping script now imports it.
Toolkit primitives (data dir, toKebabCase, normalizeToolkitId, the category
list, the *Api heuristic, docsLink→slug) existed in 2-7 copies, one pair
carrying a "must stay in sync" comment. They collapse into
toolkit-docs-generator/src/shared/, which both halves can import. All seven
data-dir consumers now honor TOOLKIT_DATA_DIR; previously only two did.
The Node-only path resolution lives in its own module because client
components reach the primitives through the integrations index, and a
node:* import anywhere in that graph fails the webpack browser build.
The consumer-side contract was a four-field duck-check that never verified
tools was an array, followed by an unchecked cast — while toToolkitSummary
immediately calls data.tools.map(). The generator's Zod schemas are now the
single shared contract, the 522-line hand-written mirror is z.infer, and
zod moves to dependencies because it enters the app's runtime path.
Corruption is now loud and absence stays quiet: three catch blocks treated
missing, unparseable, and schema-invalid identically, so a malformed file
from the nightly PR silently dropped a toolkit and 404'd. Unparseable or
invalid now throws with the file path and the Zod issues, failing the
build. An unrecognized category throws instead of being coerced to
"others", which had no route directory and would have made every toolkit
in a new category a clickable card pointing at a 404.
One cache()-wrapped loader replaces 11 full passes over the data directory
per build, and removes the scan-every-file miss path that a burst of
unknown IDs could otherwise trigger at request time.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
main squash-merged #1105 as ab9a282, and this branch predated the last five commits of that PR, so the merge was a real three-way conflict across 12 files rather than squash noise. Resolution rule: main wins on the data-contract and runtime fixes it added; S2 wins on the check enablement it exists to deliver. - main wins wholesale on the 5 files S2 never touched: the toolkit data cache keying, the cwd-anchored data-dir resolver, the sitemap resolver share, the Slack alert newlines, and the page-factory comment. - S2 wins on the .js import specifiers in scripts/. The generator project is module: NodeNext with declaration + outDir, so it emits and .ts specifiers are illegal there. main never caught this because its tsconfig only included src/**. - S2 wins on docsLink ?? null: ToolkitSlugSource declares docsLink?: string | null and the project sets exactOptionalPropertyTypes, so passing undefined is an error. Behaviour is unchanged. - sync-toolkit-sidebar.test.ts took both sides: S2's required navGroup field plus main's corrected categories, since the removed "others" catch-all no longer exists. Root and generator typecheck clean, 801 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The generator was configured like a publishable library — NodeNext with declaration, sourceMap, and outDir — but it is not one. It has no package.json, nothing imports its dist/, and every entry point runs through tsx. The only practical effect of that config was requiring .js import specifiers pointing at build output nobody consumes, which read as stray JavaScript in a TypeScript codebase. Switch to the root project's convention (module esnext, moduleResolution bundler, noEmit) and drop the .js suffix from 235 relative specifiers across src/, scripts/, and tests/. The generator's stricter flags (exactOptionalPropertyTypes, noUncheckedIndexedAccess, and the rest) are kept — this changes how modules resolve, not how strictly they check. Verified: pnpm typecheck (all three configs), pnpm lint, pnpm build, and 801 tests all pass. Runtime resolution proven under tsx by executing the nightly CLI entry point, the metadata report, and validate-merge — the last two against real committed data. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Every TypeScript entry point in the repo — 14 of them, including the nightly toolkit generation and the i18n sync — ran through 'pnpm dlx tsx'. dlx resolves the package from the registry on each invocation, so the runtime executing the build was whatever tsx had published most recently, outside the lockfile and unreviewed. Add tsx to devDependencies and call the installed binary. Note that 'pnpm exec' is not a drop-in replacement for 'pnpm dlx': exec resets the working directory to the project root. The generate step sets working-directory to toolkit-docs-generator and passes paths relative to it, so that step invokes ../node_modules/.bin/tsx by path instead. The remaining call sites already run from the repo root, where pnpm exec is correct. A test pins the invocation and asserts pnpm dlx does not reappear. Verified: pnpm typecheck, pnpm lint, pnpm build, and 801 tests pass; the nightly CLI, the sidebar sync, metadata-report, check-meta, and check-redirects were each executed under the pinned binary. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
teallarson
added a commit
that referenced
this pull request
Aug 6, 2026
These scripts live in scripts/, which nothing type-checks today — #1106 widens the generator project to cover it. Under that project's exactOptionalPropertyTypes and noUncheckedIndexedAccess they do not compile, so #1106 landing would break this PR. - IMetadataSource was imported from src/sources/interfaces, which does not export it; it lives in src/sources/internal, where all eight other consumers import it from. It is an 'import type', so it erased at runtime and no test could catch it. - Bind the indexed argv reads before use: a truthiness check on argv[i + 1] does not narrow a later, separate read of the same index. - Declare the two optional properties that legitimately receive an explicit undefined as '| undefined' rather than spreading conditionally at each call site. No behaviour change; the harness still reproduces all 117 toolkits. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
teallarson
added a commit
that referenced
this pull request
Aug 6, 2026
scripts/ is not type-checked today; #1106 widens the generator project to cover it, and under exactOptionalPropertyTypes plus noUncheckedIndexedAccess this PR's files do not compile. Fixing it here keeps the PR green whichever order the two land in. - isNonEmptyArray returned boolean, so it narrowed nothing and the caller compensated with an 'as DocumentationChunk[]' cast. Making it a type predicate narrows properly and lets the cast go. - previousToolkit is declared optional but --force-regenerate and --overwrite-output pass it explicitly as undefined, which exactOptionalPropertyTypes rejects. The type now says '| undefined', matching what callers actually do — that explicit undefined is the exact scenario the prose regression test covers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Its two path defaults were bare relative strings, so they only resolved when the script was invoked from toolkit-docs-generator/ — while the neighbouring scripts resolve through the shared data-dir resolver, which anchors on the repo root. Same directory, two conventions, and each script usable from only one place. Anchor on import.meta.url, matching validate-merge, sync-toolkit-sidebar, and verify-toolkit-join. Both defaults remain overridable by flag. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
teallarson
marked this pull request as ready for review
August 6, 2026 21:20
teallarson
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 60d274b. Configure here.
| -X POST \ | ||
| -H "Content-Type: application/json" \ | ||
| --data "$payload" \ | ||
| "${{ secrets.SLACK_PROJ_DOCS_WEBHOOK_URL }}" |
There was a problem hiding this comment.
Slack warn fails successful runs
Medium Severity
The new Slack warning step uses curl --fail-with-body without continue-on-error, causing the generate job to fail if the Slack notification itself fails. This can happen even if doc generation succeeded (e.g., with --preserve-last-known-good), leading to a misleading "generation failed" alert.
Reviewed by Cursor Bugbot for commit 60d274b. Configure here.
| return result; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Preserve flag skips provider merges
Medium Severity
--preserve-last-known-good recovery for missing design-system metadata lives only in mergeToolkitEntry, but provider-mode generation calls DataMerger.mergeToolkit directly. With that flag set, a provider run can still merge with getDefaultMetadata instead of preserving prior output or omitting a new toolkit.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 60d274b. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Several checks existed in the docs repository but were either not wired into CI or did not cover the files they appeared to cover. This PR makes the generator pipeline fail loudly when its types, tests, module resolution, metadata, or pre-commit tooling drift.
That guardrail is the foundation for the pipeline work in #1112 and #1113: source extraction and curation changes can now be reviewed against enforced checks instead of local convention.
How it fits
The PR strengthens the verification layer around the existing data flow; it does not change the product API.
Changes
pnpm typecheckcovering app code, root tests, and generator source, scripts, and tests.--require-completefailing explicitly for incomplete data.Verification
pnpm typecheck,pnpm lint,pnpm build, and 801 tests pass.tsxagainst committed data.main; refactor: singular + enforced toolkit data contract #1105 is already merged.This is the pipeline guardrail PR. #1112 adds durable curation input, and #1113 adds output-parity verification.
Note
Medium Risk
CI now blocks on type errors across app and generator; merger and CLI behavior around incomplete metadata and nightly doc generation changed, which affects what gets published when upstream catalog data is missing.
Overview
Turns on enforced static checking across the docs app, root tests, and toolkit-docs-generator by adding a root
pnpm typecheck(threetsc --noEmitprojects) and running it in the main Test workflow alongside lint, build, and tests.Tooling consistency:
tsxis added as a locked dev dependency; repo scripts and workflows move frompnpm dlx tsxtopnpm exec tsx, and the nightly toolkit-docs job invokes the generator via../node_modules/.bin/tsxso paths and the TypeScript runtime stay on the lockfile. Generatortsconfigswitches to bundler resolution withnoEmit, imports drop.jsextensions, and the separate generator Vitest config is removed in favor of root tests using@/paths.Pre-commit: The merge/rebase-aware stash-and-Ultracite block is removed; formatting now relies on
lint-stagedonly so failures propagate without touching unstaged work.Generator metadata behavior:
--require-completeno longer silently skips toolkits missing design-system metadata—it throws viaassertRequireCompleteMetadatanaming every affected toolkit. Optional--preserve-last-known-goodkeeps prior JSON on recoverable failures, omits brand-new broken toolkits from output, and the automation workflow posts Slack warnings for preserved/omitted toolkits. Placeholder metadata fromgetDefaultMetadataisisHidden: truewith integration-styledocsLink, and merge results exposeusedDefaultMetadata/recoveryfor logging.Reviewed by Cursor Bugbot for commit 60d274b. Bugbot is set up for automated code reviews on this repo. Configure here.