From 23e79f6b7c569803203ad5cc5cb1cddbb019d33b Mon Sep 17 00:00:00 2001 From: Shevchik Igor Date: Thu, 27 Aug 2026 07:59:41 +0000 Subject: [PATCH 1/3] feat(skills): compile the code fences in skill files, and fix what that found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `docs:typecheck-blocks` has compiled the fences under `docs/content/**` since #109 — 155 blocks, a CI gate. Nothing equivalent covered `skills/*/SKILL.md`, though those are what an AI agent reads BEFORE writing code: a broken snippet there is not a page someone might misread, it is a template that gets reproduced. #401 is the proof — the skills taught `LoggerBrowser`, removed in 3.0.0, for months, and hand review never caught it. This adds `skills:typecheck-blocks` over 68 fences, sharing the engine with the docs gate rather than copying it: extraction, `// @check-ignore`, the mapping of a tsc diagnostic back to `file:line:col`, and the GitHub annotation escaping now live once in `_typecheck-blocks.mjs`. `docs-typecheck` still reports 155 blocks, 0 errors — behaviour unchanged. It also generalises where the current guards do not. The substring checks in `recipe-hygiene.unit.spec.ts` catch one named symbol each, so every deprecation needs a new guard; a compiler covers the whole surface at once, which matters for #277 and its 22-symbol removal list. WHAT THE FIRST RUN FOUND, on files reviewed by hand many times: - `B24HelperManager` is not exported. The documentation gives it its own page, the helpers skill teaches constructing it directly for backend code, and `import { B24HelperManager } from '@bitrix24/b24jssdk'` resolved to nothing — confirmed `undefined` in the built bundle. Now exported (additive; the #383 reference gate correctly demanded the row, 96 -> 97). - `helper.license` / `helper.payment` do not exist — they are `licenseInfo` / `paymentInfo`. The skill's own frontmatter advertises license and payment. - `destroyB24Helper` imported from the package root; it is a member of `useB24Helper()`. - `Text.toB24Format(...)` resolving to the DOM `Text`, in two files, because the fence never imported the SDK's `Text`. Silent: the name exists, so nothing complained until a method was called on it. - `selectAccess({})` where the parameter is `string[]`; CRM ids written as `'D_42'` where the type is `number[]`; `getJsonObject()`, which takes no type argument; a `call.make()` with no type argument reading `.item` off `unknown`. - Fragments that were not TypeScript at all — `filter: { … }` as a statement, a try/catch `return`ing outside any function. Made complete rather than ignored: `const params = { filter: { … } }` also shows the reader where `filter` lives. The 24 missing imports were inserted by a throwaway script driven by the gate's own output, not by hand. Two things about the harness worth knowing: `.skills-typecheck/globals.d.ts` never declares a real SDK export. If a fence uses `Text` or `AjaxResult`, it must import it — an agent copying the snippet needs that line, and an ambient would make the gate certify code that does not run. Placeholders belonging to the reader's own application, and framework globals that really are auto-imported, are declared there. The header says so. An empty sweep is an ERROR, not a pass. A glob that stops matching would otherwise report "0 errors" and read as healthy. Also fixes a test that could not fail: `docs-typecheck.test.mjs` carried an inline re-implementation of `extractTsBlocks`, with a comment claiming it "mirrors the production logic exactly". Sixteen tests were therefore passing against a copy, so a bug in the real extractor could not redden any of them. It is exported now, and the tests bind to it — verified by breaking the real extractor, which fails 9 of them and previously failed none. Closes #402 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F22e2ft66y7nuBJjzdThBr --- .github/contributing/maintenance.md | 40 ++++ .gitignore | 1 + .skills-typecheck/globals.d.ts | 111 ++++++++++ .skills-typecheck/tsconfig.json | 27 +++ docs/content/docs/3.api-reference/1.index.md | 1 + package.json | 3 +- packages/jssdk/src/index.ts | 6 + scripts/__tests__/docs-typecheck.test.mjs | 57 ++--- .../skills-typecheck-blocks.test.mjs | 97 +++++++++ scripts/_typecheck-blocks.mjs | 196 ++++++++++++++++++ scripts/docs-typecheck.mjs | 189 +---------------- scripts/skills-typecheck-blocks.mjs | 55 +++++ skills/b24jssdk-core/SKILL.md | 40 ++-- skills/b24jssdk-filtering/SKILL.md | 29 ++- skills/b24jssdk-frame-ui/SKILL.md | 7 +- skills/b24jssdk-helpers/SKILL.md | 12 +- skills/b24jssdk-recipes/SKILL.md | 8 +- skills/b24jssdk-rest/SKILL.md | 50 +++-- 18 files changed, 650 insertions(+), 279 deletions(-) create mode 100644 .skills-typecheck/globals.d.ts create mode 100644 .skills-typecheck/tsconfig.json create mode 100644 scripts/__tests__/skills-typecheck-blocks.test.mjs create mode 100644 scripts/_typecheck-blocks.mjs create mode 100644 scripts/skills-typecheck-blocks.mjs diff --git a/.github/contributing/maintenance.md b/.github/contributing/maintenance.md index c32345bb..e4ec972a 100644 --- a/.github/contributing/maintenance.md +++ b/.github/contributing/maintenance.md @@ -216,3 +216,43 @@ Before propagating a new operator / endpoint / pattern from VibeCode docs into a | Is X deprecated? | `packages/jssdk/README-AI.md` "Deprecation notice" + JSDoc `@deprecated` markers on classes | If the SDK source disagrees with the VibeCode docs, the SDK source wins. + +## 8. The fence typecheck + +`pnpm run skills:typecheck-blocks` compiles every ` ```ts ` fence in +`skills/*/SKILL.md` against the built SDK types, the way `docs:typecheck-blocks` +has guarded the documentation site since #109. It runs inside `pnpm run +typecheck`, so the commit protocol in §5 already covers it — but know what it is +for, because it changes how a skill edit should be written. + +**It is the reason the table in §7 is a fallback rather than the first move.** A +claim you can express as code belongs in a fence, where the compiler checks it +against the real signatures on every run. The table is for what a compiler +cannot see: whether a portal supports a method, what a REST field means. + +The first run of this gate, on skills that had been reviewed by hand many times, +found: a class the documentation gives its own page to and the package did not +export (`B24HelperManager`); `helper.license` / `helper.payment`, which do not +exist — the properties are `licenseInfo` / `paymentInfo`; `destroyB24Helper` +imported from the package root when it is a member of `useB24Helper()`; +`Text.toB24Format(...)` silently resolving to the DOM `Text` because the fence +never imported the SDK's; `selectAccess({})` where the parameter is a `string[]`; +and CRM ids written as `'D_42'` where the type is `number[]`. + +### Writing a fence that the gate can check + +- **Import what you use.** An SDK export is never ambient — see the header of + `.skills-typecheck/globals.d.ts` for why. An agent copying the fence needs the + import, so the fence must carry it. +- **Fragments are declarations too.** `filter: { … }` alone is not TypeScript; + `const params = { filter: { … } }` is, and it also shows the reader where + `filter` actually lives. +- **Placeholders belong in `globals.d.ts`** — a domain type, a handler the + reader writes, an id list they already have. +- **`// @check-ignore` on the line before the fence** skips it. Use it when + making the snippet compile would misrepresent it, and say why in the marker. + It is the last resort: every ignored fence is a fence the gate stops checking. + +The gate proves a fence type-checks. It does not prove the method exists on a +portal, or that the field names are right — that is what `skills:verify` (#113) +is for. diff --git a/.gitignore b/.gitignore index f5ccad40..ad64abda 100644 --- a/.gitignore +++ b/.gitignore @@ -110,4 +110,5 @@ coverage # docs-typecheck generated temp files (scripts/docs-typecheck.mjs) .docs-typecheck/tmp/ +.skills-typecheck/tmp/ __pycache__/ diff --git a/.skills-typecheck/globals.d.ts b/.skills-typecheck/globals.d.ts new file mode 100644 index 00000000..4c0a039b --- /dev/null +++ b/.skills-typecheck/globals.d.ts @@ -0,0 +1,111 @@ +/** + * Ambient declarations for the TypeScript fences in `skills/*\/SKILL.md` (#402). + * + * Skill files are reference prose, so many fences are deliberately fragments: + * they show the call under discussion, not a runnable program. Without ambients + * every such fence would fail on a name the surrounding paragraph established, + * and the gate would report noise instead of defects. + * + * The line drawn here, which is the whole design of this file: + * + * - A REAL export of `@bitrix24/b24jssdk` is NEVER declared here. If a fence + * uses `Text`, `AjaxResult` or `EnumCrmEntityTypeId`, the fence must import + * it — an agent copying that snippet needs the import, and hiding it behind + * an ambient would make the gate certify code that does not run. The first + * run of this gate found `Text.toB24Format(...)` resolving to the DOM `Text` + * for exactly that reason. + * - A PLACEHOLDER belonging to the reader's own application — a domain type, a + * handler they write, an id list they already have — is declared here. + * - A framework global that really is auto-imported at the callsite (Nuxt's + * `useNuxtApp`, `navigateTo`) is declared here. + * + * This mirrors `.docs-typecheck/globals.d.ts`, including its known limitation: + * an ambient cannot tell whether the reader actually has that binding, so the + * gate proves a fence type-checks, not that it is complete. Keep the list short + * for that reason — every entry is a check the gate stops performing. + */ + +// region The reader's own application //// + +// Domain types used as `` arguments in examples. Deliberately loose: the +// examples illustrate the call shape, not any particular portal's schema. +type CrmItem = Record +type Contact = Record +type ChatInfo = Record +type EventLogItem = Record +type Deal = Record +type TaskItem = Record +type EventItem = Record + +// Values and functions the surrounding prose establishes the reader already has. +declare const ids: number[] +declare const arrayOfCalls: any[] +declare const b24OAuthParams: any +declare const clientId: string +declare const clientSecret: string +declare const db: any +declare const sixMonthsAgo: Date +declare const rootEl: HTMLElement +declare function processDeal(item: any): void +declare function handleCrmEvent(message: any): void +declare function handleImEvent(message: any): void +declare function handleAppEvent(message: any): void +declare function routeForPlace(place: string): string +declare function isSamePath(a: string, b: string): boolean +declare function risky(): Promise +declare function main(): Promise + +// endregion //// + +// region Instances a previous fence constructed //// + +// The live SDK client, as in `.docs-typecheck/globals.d.ts`. `let`, not `const`: +// frame snippets assign to it via `$b24 = await initializeB24Frame()`, which a +// `declare const` would reject with TS2588. +declare let $b24: import('@bitrix24/b24jssdk').B24Frame + +// `helper` and `logger` are built in an earlier fence on the same page and then +// used across several later ones. Same concession `.docs-typecheck` makes for +// `$b24`: the alternative is repeating the constructor in every fragment, which +// buries the call being explained. +declare const helper: ReturnType extends infer H + ? H extends { getB24Helper: () => infer M } ? M : any + : any +declare const logger: import('@bitrix24/b24jssdk').LoggerInterface + +// Destructured from `useB24Helper()` in the page's first fence. +declare const usePullClient: () => void +declare const useSubscribePullClient: (callback: (message: any) => void, moduleId?: string) => () => void +declare const startPullClient: () => void + +// endregion //// + +// region Framework globals //// + +// Nuxt auto-imports these in an app; a fence showing app code does not import them. +declare function useNuxtApp(): any +declare function useRouter(): any +declare function navigateTo(to: any, options?: any): any +declare function onNuxtReady(callback: () => void): void +declare function defineNuxtRouteMiddleware(handler: (to: any, from: any) => any): any + +// endregion //// + +// Single-file components are out of scope for a type gate over Markdown; a +// fence importing from 'vue' only needs the names to resolve. +declare module 'vue' { + export function ref(value: T): { value: T } + export function computed(getter: () => T): { value: T } + export function onMounted(callback: () => void): void + export function onUnmounted(callback: () => void): void + export function onBeforeUnmount(callback: () => void): void + export function watch(source: any, callback: any, options?: any): () => void +} + +// Vite/Nuxt extend ImportMeta; the DOM lib does not know about these. +interface ImportMeta { + dev?: boolean + server?: boolean + client?: boolean + env?: Record +} diff --git a/.skills-typecheck/tsconfig.json b/.skills-typecheck/tsconfig.json new file mode 100644 index 00000000..40a70f20 --- /dev/null +++ b/.skills-typecheck/tsconfig.json @@ -0,0 +1,27 @@ +{ + "$schema": "https://json.schemastore.org/tsconfig", + "compilerOptions": { + "target": "ESNext", + "module": "ESNext", + "moduleResolution": "bundler", + "lib": [ + "ESNext", + "DOM" + ], + "strict": false, + "noImplicitAny": false, + "skipLibCheck": true, + "noEmit": true, + "moduleDetection": "force", + "allowJs": false, + "allowImportingTsExtensions": false, + "isolatedModules": false, + "types": [ + "node" + ] + }, + "include": [ + "globals.d.ts", + "tmp/**/*.ts" + ] +} \ No newline at end of file diff --git a/docs/content/docs/3.api-reference/1.index.md b/docs/content/docs/3.api-reference/1.index.md index 9ab1645a..a01255f1 100644 --- a/docs/content/docs/3.api-reference/1.index.md +++ b/docs/content/docs/3.api-reference/1.index.md @@ -99,6 +99,7 @@ Aggregated portal state — profile, app, licence, currency. | Export | What it is | Guide | | --- | --- | --- | +| [`B24HelperManager`](https://github.com/bitrix24/b24jssdk/blob/main/packages/jssdk/src/helper/helper-manager.ts) | Loads and caches portal-wide data (profile, app, currencies, options, license, payment) and owns the Pull-client glue. Construct it directly for backend code; in an app prefer the composable below. | [guide](/docs/working-with-the-rest-api/helper/) | | [`useB24Helper`](https://github.com/bitrix24/b24jssdk/blob/main/packages/jssdk/src/helper/use-b24-helper.ts) | Composable that initialises and exposes the helper. | [guide](/docs/working-with-the-rest-api/helper-use-b24-helper/) | | [`LoadDataType`](https://github.com/bitrix24/b24jssdk/blob/main/packages/jssdk/src/types/b24-helper.ts) | Which data sets the helper loads. | [guide](/docs/working-with-the-rest-api/helper/) | | [`EnumAppStatus`](https://github.com/bitrix24/b24jssdk/blob/main/packages/jssdk/src/types/b24-helper.ts) | Application status values. | [guide](/docs/working-with-the-rest-api/helper-app-manager/) | diff --git a/package.json b/package.json index 21e24ba4..7e69f0a9 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ "docs:preview": "nuxt preview docs", "docs:lint": "pnpm run docs:lint-pages && pnpm run docs:lint-links && pnpm run docs:lint-api-index", "docs:typecheck": "pnpm --filter ./docs typecheck", + "skills:typecheck-blocks": "node scripts/skills-typecheck-blocks.mjs", "docs:typecheck-blocks": "node scripts/docs-typecheck.mjs", "skills:install": "pnpm --dir skills/b24jssdk-recipes install --frozen-lockfile", "skills:typecheck": "pnpm run skills:install && tsc -p skills/b24jssdk-recipes/tsconfig.json", @@ -75,7 +76,7 @@ "docs:lint-links": "node scripts/docs-link-check.mjs", "docs:lint-api-index": "node scripts/check-api-reference-index.mjs", "docs:lint:test": "node --test \"scripts/__tests__/**/*.test.mjs\"", - "typecheck": "pnpm run package-jssdk:typecheck && pnpm run package-jssdk-nuxt:typecheck && pnpm run docs:typecheck && pnpm run playground-nuxt:typecheck && pnpm run playground-cli:typecheck && pnpm run docs:typecheck-blocks && pnpm run skills:typecheck && pnpm run contributing:typecheck", + "typecheck": "pnpm run package-jssdk:typecheck && pnpm run package-jssdk-nuxt:typecheck && pnpm run docs:typecheck && pnpm run playground-nuxt:typecheck && pnpm run playground-cli:typecheck && pnpm run docs:typecheck-blocks && pnpm run skills:typecheck && pnpm run skills:typecheck-blocks && pnpm run contributing:typecheck", "release:bump": "node scripts/bump-version.mjs" }, "devDependencies": { diff --git a/packages/jssdk/src/index.ts b/packages/jssdk/src/index.ts index 312c91a7..563ac438 100644 --- a/packages/jssdk/src/index.ts +++ b/packages/jssdk/src/index.ts @@ -48,6 +48,12 @@ export * from './tools/batch-ref-v3' export * from './hook/index' export * from './frame/index' export * from './oauth/index' +// `B24HelperManager` has its own documentation page and the `b24jssdk-helpers` +// skill teaches constructing it directly for backend code — but it was never +// exported, so `import { B24HelperManager } from '@bitrix24/b24jssdk'` resolved +// to nothing. Found by the skills fence typecheck (#402); additive, so no +// existing code changes meaning. +export * from './helper/helper-manager' export * from './helper/use-b24-helper' export * from './pullClient/index' export * from './loader-b24frame' diff --git a/scripts/__tests__/docs-typecheck.test.mjs b/scripts/__tests__/docs-typecheck.test.mjs index 79ab3dc4..7b0cdf1c 100644 --- a/scripts/__tests__/docs-typecheck.test.mjs +++ b/scripts/__tests__/docs-typecheck.test.mjs @@ -31,51 +31,18 @@ const INTEGRATION_SKIP = !existsSync(TSC_BIN) || !existsSync(SDK_TYPES) ? 'requires pnpm install && pnpm run dev:prepare' : false -// Import extractTsBlocks directly for unit testing. -// The function is not exported, so we test it via a thin inline re-implementation -// that mirrors the production logic exactly (keeping tests independent of internals). -function extractTsBlocks(content, filePath = 'test.md') { - const fileLines = content.replace(/\r\n/g, '\n').split('\n') - const blocks = [] - let inFence = false - let fenceLen = 0 - let blockLines = [] - let blockStart = 0 - let skip = false - - for (let i = 0; i < fileLines.length; i++) { - const line = fileLines[i] - - if (!inFence) { - const match = line.match(/^(`{3,})(typescript|ts)(?:\s+\[.*?\])?\s*$/) - if (!match) continue - fenceLen = match[1].length - inFence = true - blockLines = [] - blockStart = i + 2 - - let prev = i - 1 - while (prev >= 0 && fileLines[prev].trim() === '') prev-- - skip = prev >= 0 && fileLines[prev].trim().startsWith('// @check-ignore') - continue - } - - const close = line.match(/^(`{3,})\s*$/) - if (close && close[1].length >= fenceLen) { - if (!skip && blockLines.length > 0) { - blocks.push({ lines: [...blockLines], startLine: blockStart, filePath }) - } - inFence = false - skip = false - blockLines = [] - continue - } - - if (!skip) blockLines.push(line) - } - - return blocks -} +// The REAL extractor, imported from the shared module. +// +// Until #402 this file carried an inline re-implementation, with a comment +// saying it "mirrors the production logic exactly (keeping tests independent of +// internals)". That is the wrong trade: 16 tests were passing against a copy, so +// a bug introduced in the production extractor could not fail any of them — +// which is precisely what a test of it is for. `extractTsBlocks` is exported now +// that the engine is shared between the docs and skills gates, so the copy is +// gone and these tests bind to the code that actually runs. +import { extractTsBlocks as extract } from '../_typecheck-blocks.mjs' + +const extractTsBlocks = (content, filePath = 'test.md') => extract(content, filePath) // ── extractTsBlocks unit tests ──────────────────────────────────────────── diff --git a/scripts/__tests__/skills-typecheck-blocks.test.mjs b/scripts/__tests__/skills-typecheck-blocks.test.mjs new file mode 100644 index 00000000..ddc4cc61 --- /dev/null +++ b/scripts/__tests__/skills-typecheck-blocks.test.mjs @@ -0,0 +1,97 @@ +#!/usr/bin/env node +// Tests for scripts/skills-typecheck-blocks.mjs (#402). +// +// Run with: node --test scripts/__tests__/skills-typecheck-blocks.test.mjs +// +// The extraction and reporting logic is shared with the docs gate and is +// covered by docs-typecheck.test.mjs. What is specific here — and what these +// tests pin — is the wiring: which files the gate discovers, and that it treats +// an empty sweep as a failure rather than a pass. + +import { existsSync, readdirSync } from 'node:fs' +import { join, resolve, dirname } from 'node:path' +import { fileURLToPath } from 'node:url' +import { spawnSync } from 'node:child_process' +import test from 'node:test' +import assert from 'node:assert/strict' + +const __dirname = dirname(fileURLToPath(import.meta.url)) +const REPO_ROOT = resolve(__dirname, '..', '..') +const SCRIPT = resolve(__dirname, '..', 'skills-typecheck-blocks.mjs') + +// The gate needs tsc and the built SDK types. Same convention as the docs test: +// skip the spawning tests when the prerequisites are absent, since the docs-lint +// CI job runs this file without `dev:prepare`. Real coverage comes from the +// `typecheck` job, which runs the gate for real. +const TSC_BIN = join(REPO_ROOT, 'node_modules', 'typescript', 'bin', 'tsc') +const SDK_TYPES = join(REPO_ROOT, 'packages', 'jssdk', 'dist', 'esm', 'index.d.ts') +const SKIP = !existsSync(TSC_BIN) || !existsSync(SDK_TYPES) + +test('every skill directory is covered, not a hand-written list', () => { + // The failure this guards is specific: #401 shipped a hand-maintained list of + // three skill files while the tree held seven, and nothing noticed. Asserting + // "the files it checks" == "the SKILL.md files that exist" is the only form + // that cannot go stale. + const onDisk = readdirSync(join(REPO_ROOT, 'skills'), { withFileTypes: true }) + .filter(entry => entry.isDirectory() || entry.isSymbolicLink()) + .map(entry => join('skills', entry.name, 'SKILL.md')) + .filter(rel => existsSync(join(REPO_ROOT, rel))) + + assert.ok(onDisk.length > 0, 'no SKILL.md found — the layout changed') + + const result = spawnSync(process.execPath, [SCRIPT], { + cwd: REPO_ROOT, + encoding: 'utf8', + env: { ...process.env, GITHUB_ACTIONS: '' } + }) + const output = (result.stdout ?? '') + (result.stderr ?? '') + + if (SKIP) { + // Without dist/ the script exits early with the actionable dist message; + // the discovery assertion above still ran, which is the part worth pinning. + assert.match(output, /dev:prepare|not installed/) + return + } + + // The block count is reported, and it is not zero. + const match = output.match(/skills-typecheck: (\d+) block\(s\) checked/) + assert.ok(match, `expected a block count in:\n${output}`) + assert.ok(Number(match[1]) > 0, 'zero blocks checked — the sweep found nothing') +}) + +test('the skills tree currently type-checks clean', { skip: SKIP }, () => { + const result = spawnSync(process.execPath, [SCRIPT], { + cwd: REPO_ROOT, + encoding: 'utf8', + env: { ...process.env, GITHUB_ACTIONS: '' } + }) + const output = (result.stdout ?? '') + (result.stderr ?? '') + + assert.equal(result.status, 0, `gate failed:\n${output}`) + assert.match(output, /0 error\(s\)/) +}) + +test('an empty sweep is an error, not a clean run', { skip: SKIP }, () => { + // A glob that stops matching — a renamed directory, say — would otherwise + // report "0 errors" and read as healthy. Exercised through the shared engine + // with a directory that holds no Markdown at all. + const result = spawnSync( + process.execPath, + ['-e', ` + import('${resolve(__dirname, '..', '_typecheck-blocks.mjs').replace(/\\/g, '/')}') + .then(({ checkBlocks }) => { + const code = checkBlocks({ + label: 'probe', + repoRoot: ${JSON.stringify(REPO_ROOT)}, + checkDir: ${JSON.stringify(join(REPO_ROOT, '.skills-typecheck'))}, + files: [] + }) + process.exit(code) + }) + `], + { cwd: REPO_ROOT, encoding: 'utf8' } + ) + + assert.equal(result.status, 1) + assert.match((result.stdout ?? '') + (result.stderr ?? ''), /no TS blocks found/) +}) diff --git a/scripts/_typecheck-blocks.mjs b/scripts/_typecheck-blocks.mjs new file mode 100644 index 00000000..1bfea0ac --- /dev/null +++ b/scripts/_typecheck-blocks.mjs @@ -0,0 +1,196 @@ +/** + * Shared engine for the two "compile the fenced code blocks" gates. + * + * `docs-typecheck.mjs` has checked the fences under `docs/content/**` since #109. + * `skills-typecheck-blocks.mjs` does the same for `skills/*\/SKILL.md` (#402), + * and the reason it exists is worth stating: skill files are what an AI agent + * reads BEFORE it writes code, so a broken snippet there is not a page someone + * might misread — it is a template that gets reproduced. The first run of this + * gate found a documented class that is not exported from the package at all. + * + * The two differ only in which files they walk, which ambient declarations the + * fragments may rely on, and where the scratch directory lives. Everything else + * — fence extraction, the `// @check-ignore` marker, mapping a `tsc` diagnostic + * back to `file:line:col` in the Markdown, the GitHub annotation escaping — is + * identical, and lived in one copy before this split. It stays in one copy. + */ + +import { readFileSync, writeFileSync, mkdirSync, rmSync, existsSync } from 'node:fs' +import { join, relative, basename } from 'node:path' +import { spawnSync } from 'node:child_process' + +const IS_CI = process.env.GITHUB_ACTIONS === 'true' + +// GitHub Actions workflow commands use %25/%0D/%0A as escape sequences. +// Without escaping, a tsc message containing a literal newline could inject +// additional workflow commands (e.g. ::set-env::, ::add-mask::). +function escapeAnnotation(s) { + return s.replace(/%/g, '%25').replace(/\r/g, '%0D').replace(/\n/g, '%0A') +} + +/** + * Extract ```ts / ```typescript fenced blocks from a markdown file. + * + * Skips: + * - ```ts-type fences (type-signature fragments, not executable code) + * - Blocks preceded by // @check-ignore (optionally "// @check-ignore: reason") + * on the nearest non-empty line above the fence + * + * Returns: Array of { lines: string[], startLine: number, filePath: string } + * where startLine is the 1-indexed line of the first code line in the MD file. + */ +export function extractTsBlocks(content, filePath) { + // Normalise CRLF so that line splitting and regex matching work correctly + // on files committed with Windows line endings. + const fileLines = content.replace(/\r\n/g, '\n').split('\n') + const blocks = [] + let inFence = false + let fenceLen = 0 + let blockLines = [] + let blockStart = 0 + let skip = false + + for (let i = 0; i < fileLines.length; i++) { + const line = fileLines[i] + + if (!inFence) { + // Match opening fence: ```ts or ```typescript, with optional [filename] annotation. + // Explicitly exclude ```ts-type (type-signature fragments). + const match = line.match(/^(`{3,})(typescript|ts)(?:\s+\[.*?\])?\s*$/) + if (!match) continue + + fenceLen = match[1].length + inFence = true + blockLines = [] + // blockStart is the 1-indexed line of the first code line (line after the fence). + blockStart = i + 2 + + // Check for // @check-ignore (optionally "// @check-ignore: reason") on + // the nearest preceding non-empty line. + let prev = i - 1 + while (prev >= 0 && fileLines[prev].trim() === '') prev-- + skip = prev >= 0 && fileLines[prev].trim().startsWith('// @check-ignore') + continue + } + + // Inside fence — check for matching closing fence. + const close = line.match(/^(`{3,})\s*$/) + if (close && close[1].length >= fenceLen) { + if (!skip && blockLines.length > 0) { + blocks.push({ lines: [...blockLines], startLine: blockStart, filePath }) + } + inFence = false + skip = false + blockLines = [] + continue + } + + if (!skip) blockLines.push(line) + } + + return blocks +} + +/** + * Compile every fenced block in `files` and report diagnostics against the + * original Markdown coordinates. + * + * @param {object} options + * @param {string} options.label prefix for every log line, e.g. `docs-typecheck` + * @param {string} options.repoRoot absolute repo root + * @param {string} options.checkDir scratch dir holding tsconfig.json + globals.d.ts + * @param {string[]} options.files absolute paths of the Markdown files to walk + * @returns {number} process exit code — 0 clean, 1 on any error + */ +export function checkBlocks({ label, repoRoot, checkDir, files }) { + const tmpDir = join(checkDir, 'tmp') + const tsconfigPath = join(checkDir, 'tsconfig.json') + const tscBin = join(repoRoot, 'node_modules', 'typescript', 'bin', 'tsc') + let errors = 0 + + function logError(mdFile, mdLine, col, code, message) { + const relFile = relative(repoRoot, mdFile) + console.log(`\u001B[31mERROR\u001B[0m ${relFile}:${mdLine}:${col} ${code}: ${message}`) + if (IS_CI) { + process.stdout.write( + `::error file=${escapeAnnotation(relFile)},line=${mdLine},col=${col}::${escapeAnnotation(code)}: ${escapeAnnotation(message)}\n` + ) + } + errors++ + } + + if (!existsSync(tscBin)) { + console.error(`\u001B[31mERROR\u001B[0m ${label}: TypeScript not installed — run \`pnpm install\``) + return 1 + } + + // Clean and recreate the tmp directory on every run. + if (existsSync(tmpDir)) rmSync(tmpDir, { recursive: true }) + mkdirSync(tmpDir, { recursive: true }) + + /** @type {Map} */ + const blockMap = new Map() + let blockIndex = 0 + + for (const file of files) { + const content = readFileSync(file, 'utf8') + for (const block of extractTsBlocks(content, file)) { + const name = `block-${String(blockIndex).padStart(4, '0')}.ts` + writeFileSync(join(tmpDir, name), block.lines.join('\n') + '\n', 'utf8') + blockMap.set(name, { filePath: file, startLine: block.startLine }) + blockIndex++ + } + } + + if (blockIndex === 0) { + // Not a pass. An empty sweep means the glob stopped matching — a renamed + // directory, say — and reporting "0 errors" for it would read as healthy. + console.error(`\u001B[31mERROR\u001B[0m ${label}: no TS blocks found — the file list is empty, which is almost certainly a broken path rather than a clean tree`) + return 1 + } + + const result = spawnSync( + process.execPath, + [tscBin, '--noEmit', '-p', tsconfigPath], + { cwd: repoRoot, encoding: 'utf8' } + ) + + // tsc writes diagnostics to stdout; combine both streams to be safe. + const output = (result.stdout ?? '') + (result.stderr ?? '') + + // Parse: path/to/block-XXXX.ts(LINE,COL): error|warning TSNNNN: message + // tsc sometimes emits continuation lines (indented) after the primary + // diagnostic — collect them so the full message is reported. + const DIAG_RE = /^(.+\.ts)\((\d+),(\d+)\): (error|warning) (TS\d+): (.+)$/ + const outputLines = output.split('\n') + + for (let i = 0; i < outputLines.length; i++) { + const m = outputLines[i].match(DIAG_RE) + if (!m) continue + const [, rawPath, lineStr, colStr, level, code, firstMessage] = m + if (level === 'warning') continue // warnings-only: skip for now + + // Collect indented continuation lines (type expansion details, etc.) + let message = firstMessage + while (i + 1 < outputLines.length && /^\s+/.test(outputLines[i + 1])) { + i++ + message += ' ' + outputLines[i].trim() + } + + const block = blockMap.get(basename(rawPath)) + if (!block) { + // Error in globals.d.ts or an untracked file — surface it as infrastructure noise. + console.error(`\u001B[31mERROR\u001B[0m ${label}: infrastructure error — ${rawPath}: ${code}: ${message}`) + errors++ + continue + } + const mdLine = block.startLine + (Number.parseInt(lineStr, 10) - 1) + logError(block.filePath, mdLine, Number.parseInt(colStr, 10), code, message) + } + + // Remove tmp on success; keep on failure for local debugging. + if (errors === 0) rmSync(tmpDir, { recursive: true }) + + console.log(`\n${label}: ${blockIndex} block(s) checked, ${errors} error(s)`) + return errors > 0 ? 1 : 0 +} diff --git a/scripts/docs-typecheck.mjs b/scripts/docs-typecheck.mjs index c008ca02..d1a3c5ce 100644 --- a/scripts/docs-typecheck.mjs +++ b/scripts/docs-typecheck.mjs @@ -16,195 +16,28 @@ * - // @check-ignore on the line immediately before a ```ts fence skips that block. * - ```ts-type fences (type signature fragments) are never checked. * + * The extraction and reporting live in `_typecheck-blocks.mjs`, shared with the + * skills gate (#402) so the two cannot drift. + * * Prerequisites: pnpm install && pnpm run dev:prepare (creates dist/ for jssdk types). */ -import { readFileSync, writeFileSync, mkdirSync, rmSync, existsSync } from 'node:fs' -import { join, resolve, relative, dirname, basename } from 'node:path' +import { resolve, dirname, join } from 'node:path' import { fileURLToPath } from 'node:url' -import { spawnSync } from 'node:child_process' import { walkMarkdownFiles } from './_docs-utils.mjs' import { requireSdkTypes } from './_require-sdk-types.mjs' +import { checkBlocks } from './_typecheck-blocks.mjs' const __dirname = dirname(fileURLToPath(import.meta.url)) const REPO_ROOT = resolve(__dirname, '..') -const DOCS_ROOT = join(REPO_ROOT, 'docs', 'content', 'docs') -const CHECK_DIR = join(REPO_ROOT, '.docs-typecheck') -const TMP_DIR = join(CHECK_DIR, 'tmp') -const TSCONFIG_PATH = join(CHECK_DIR, 'tsconfig.json') -const TSC_BIN = join(REPO_ROOT, 'node_modules', 'typescript', 'bin', 'tsc') // SDK types live in dist/, produced by `pnpm run dev:prepare`. Without them the // generated blocks fail with a cryptic TS2307 — guard with an actionable message (#109). requireSdkTypes('docs:typecheck-blocks') -const IS_CI = process.env.GITHUB_ACTIONS === 'true' - -let errors = 0 - -// GitHub Actions workflow commands use %25/%0D/%0A as escape sequences. -// Without escaping, a tsc message containing a literal newline could inject -// additional workflow commands (e.g. ::set-env::, ::add-mask::). -function escapeAnnotation(s) { - return s.replace(/%/g, '%25').replace(/\r/g, '%0D').replace(/\n/g, '%0A') -} - -function logError(mdFile, mdLine, col, code, message) { - const relFile = relative(REPO_ROOT, mdFile) - console.log(`\x1B[31mERROR\x1B[0m ${relFile}:${mdLine}:${col} ${code}: ${message}`) - if (IS_CI) { - process.stdout.write( - `::error file=${escapeAnnotation(relFile)},line=${mdLine},col=${col}::${escapeAnnotation(code)}: ${escapeAnnotation(message)}\n` - ) - } - errors++ -} - -/** - * Extract ```ts / ```typescript fenced blocks from a markdown file. - * - * Skips: - * - ```ts-type fences (type-signature fragments, not executable code) - * - Blocks preceded by // @check-ignore (optionally "// @check-ignore: reason") - * on the nearest non-empty line above the fence - * - * Returns: Array of { lines: string[], startLine: number, filePath: string } - * where startLine is the 1-indexed line of the first code line in the MD file. - */ -function extractTsBlocks(content, filePath) { - // Normalise CRLF so that line splitting and regex matching work correctly - // on files committed with Windows line endings. - const fileLines = content.replace(/\r\n/g, '\n').split('\n') - const blocks = [] - let inFence = false - let fenceLen = 0 - let blockLines = [] - let blockStart = 0 - let skip = false - - for (let i = 0; i < fileLines.length; i++) { - const line = fileLines[i] - - if (!inFence) { - // Match opening fence: ```ts or ```typescript, with optional [filename] annotation. - // Explicitly exclude ```ts-type (type-signature fragments). - const match = line.match(/^(`{3,})(typescript|ts)(?:\s+\[.*?\])?\s*$/) - if (!match) continue - - fenceLen = match[1].length - inFence = true - blockLines = [] - // blockStart is the 1-indexed line of the first code line (line after the fence). - blockStart = i + 2 - - // Check for // @check-ignore (optionally "// @check-ignore: reason") on - // the nearest preceding non-empty line. - let prev = i - 1 - while (prev >= 0 && fileLines[prev].trim() === '') prev-- - skip = prev >= 0 && fileLines[prev].trim().startsWith('// @check-ignore') - continue - } - - // Inside fence — check for matching closing fence. - const close = line.match(/^(`{3,})\s*$/) - if (close && close[1].length >= fenceLen) { - if (!skip && blockLines.length > 0) { - blocks.push({ lines: [...blockLines], startLine: blockStart, filePath }) - } - inFence = false - skip = false - blockLines = [] - continue - } - - if (!skip) blockLines.push(line) - } - - return blocks -} - -function main() { - if (!existsSync(TSC_BIN)) { - console.error('\x1B[31mERROR\x1B[0m docs-typecheck: TypeScript not installed — run `pnpm install`') - process.exit(1) - } - // The built SDK types are guarded once at module load by requireSdkTypes() - // above — no need to re-check here. - - // Clean and recreate the tmp directory on every run. - if (existsSync(TMP_DIR)) rmSync(TMP_DIR, { recursive: true }) - mkdirSync(TMP_DIR, { recursive: true }) - - // Walk docs and collect all TS blocks. - const files = walkMarkdownFiles(DOCS_ROOT) - /** @type {Map} */ - const blockMap = new Map() - let blockIndex = 0 - - for (const file of files) { - const content = readFileSync(file, 'utf8') - const blocks = extractTsBlocks(content, file) - for (const block of blocks) { - const name = `block-${String(blockIndex).padStart(4, '0')}.ts` - writeFileSync(join(TMP_DIR, name), block.lines.join('\n') + '\n', 'utf8') - blockMap.set(name, { filePath: file, startLine: block.startLine }) - blockIndex++ - } - } - - if (blockIndex === 0) { - console.log('docs-typecheck: no TS blocks found') - return - } - - // Run tsc. - const result = spawnSync( - process.execPath, - [TSC_BIN, '--noEmit', '-p', TSCONFIG_PATH], - { cwd: REPO_ROOT, encoding: 'utf8' } - ) - - // tsc writes diagnostics to stdout; combine both streams to be safe. - const output = (result.stdout ?? '') + (result.stderr ?? '') - - // Parse: path/to/block-XXXX.ts(LINE,COL): error|warning TSNNNN: message - // tsc sometimes emits continuation lines (indented) after the primary - // diagnostic — collect them so the full message is reported. - const DIAG_RE = /^(.+\.ts)\((\d+),(\d+)\): (error|warning) (TS\d+): (.+)$/ - const outputLines = output.split('\n') - - for (let i = 0; i < outputLines.length; i++) { - const line = outputLines[i] - const m = line.match(DIAG_RE) - if (!m) continue - const [, rawPath, lineStr, colStr, level, code, firstMessage] = m - if (level === 'warning') continue // warnings-only: skip for now - - // Collect indented continuation lines (type expansion details, etc.) - let message = firstMessage - while (i + 1 < outputLines.length && /^\s+/.test(outputLines[i + 1])) { - i++ - message += ' ' + outputLines[i].trim() - } - - const tmpName = basename(rawPath) - const block = blockMap.get(tmpName) - if (!block) { - // Error in globals.d.ts or an untracked file — surface it as infrastructure noise. - console.error(`\x1B[31mERROR\x1B[0m docs-typecheck: infrastructure error — ${rawPath}: ${code}: ${message}`) - errors++ - continue - } - const tscLine = Number.parseInt(lineStr, 10) - const mdLine = block.startLine + (tscLine - 1) - logError(block.filePath, mdLine, Number.parseInt(colStr, 10), code, message) - } - - // Remove tmp on success; keep on failure for local debugging. - if (errors === 0) rmSync(TMP_DIR, { recursive: true }) - - console.log(`\ndocs-typecheck: ${blockIndex} block(s) checked, ${errors} error(s)`) - if (errors > 0) process.exit(1) -} - -main() +process.exit(checkBlocks({ + label: 'docs-typecheck', + repoRoot: REPO_ROOT, + checkDir: join(REPO_ROOT, '.docs-typecheck'), + files: walkMarkdownFiles(join(REPO_ROOT, 'docs', 'content', 'docs')) +})) diff --git a/scripts/skills-typecheck-blocks.mjs b/scripts/skills-typecheck-blocks.mjs new file mode 100644 index 00000000..ec62894a --- /dev/null +++ b/scripts/skills-typecheck-blocks.mjs @@ -0,0 +1,55 @@ +#!/usr/bin/env node + +/** + * Type-checks the ```ts / ```typescript fences in `skills/*\/SKILL.md` (#402). + * + * Why this exists separately from the docs gate: skill files are what an AI + * agent reads BEFORE writing code, so a broken snippet there is not a page a + * human might misread — it is a template that gets reproduced. `docs-typecheck` + * has guarded the documentation site since #109; the skills had nothing, and it + * showed. #401 found them teaching `LoggerBrowser`, removed in 3.0.0, for + * months. The first run of THIS gate found a class the documentation gives its + * own page to and the package does not export at all. + * + * It also generalises: the substring guards in + * `test/integration/skills-recipes/recipe-hygiene.unit.spec.ts` catch one named + * symbol each, so every deprecation needs a new guard. A compiler covers the + * whole surface at once — which matters for #277, whose removal list is 22 + * symbols long. + * + * Extraction, `// @check-ignore` handling and diagnostic mapping are shared with + * the docs gate — see `_typecheck-blocks.mjs`. What differs is only the file + * list and `.skills-typecheck/globals.d.ts`, whose header explains what may and + * may not be declared there. + * + * Prerequisites: pnpm install && pnpm run dev:prepare (creates dist/ for jssdk types). + */ + +import { readdirSync, existsSync } from 'node:fs' +import { resolve, dirname, join } from 'node:path' +import { fileURLToPath } from 'node:url' +import { requireSdkTypes } from './_require-sdk-types.mjs' +import { checkBlocks } from './_typecheck-blocks.mjs' + +const __dirname = dirname(fileURLToPath(import.meta.url)) +const REPO_ROOT = resolve(__dirname, '..') + +requireSdkTypes('skills:typecheck-blocks') + +// Discovered, not listed. A hand-maintained list of skill files went stale once +// already (#401): four of the seven were unguarded and nobody noticed, because +// "every listed file exists" says nothing about whether the list is complete. +// `isDirectory()` reports the dirent's own type, so a symlinked skill needs the +// second test or it is silently skipped. +const SKILLS_DIR = join(REPO_ROOT, 'skills') +const files = readdirSync(SKILLS_DIR, { withFileTypes: true }) + .filter(entry => entry.isDirectory() || entry.isSymbolicLink()) + .map(entry => join(SKILLS_DIR, entry.name, 'SKILL.md')) + .filter(existsSync) + +process.exit(checkBlocks({ + label: 'skills-typecheck', + repoRoot: REPO_ROOT, + checkDir: join(REPO_ROOT, '.skills-typecheck'), + files +})) diff --git a/skills/b24jssdk-core/SKILL.md b/skills/b24jssdk-core/SKILL.md index c125eaa4..c41525ca 100644 --- a/skills/b24jssdk-core/SKILL.md +++ b/skills/b24jssdk-core/SKILL.md @@ -40,6 +40,7 @@ logger.info(`Hello, ${me.getData()!.result.NAME}`) Alternative constructor (manual parts): ```ts +import { B24Hook } from '@bitrix24/b24jssdk' const $b24 = new B24Hook({ b24Url: 'https://your_domain.bitrix24.com', userId: 1, @@ -163,24 +164,26 @@ $b24.setLogger(logger) ```ts import { AjaxError, SdkError } from '@bitrix24/b24jssdk' -try { - const res = await $b24.actions.v2.call.make<{ item: Deal }>({ - method: 'crm.item.get', - params: { entityTypeId: 2, id: 999_999 } - }) - if (!res.isSuccess) { - // Soft errors only (see softErrorCodes below). Most failures throw. - logger.warning('non-success result', { errors: res.getErrorMessages() }) - return - } - return res.getData()!.result.item -} catch (e) { - if (e instanceof AjaxError) { - logger.error('REST error', { code: e.code, status: e.status, message: e.message, requestInfo: e.requestInfo }) - } else if (e instanceof SdkError) { - logger.error('SDK error', { code: e.code, message: e.message }) - } else { - throw e +async function loadDeal() { + try { + const res = await $b24.actions.v2.call.make<{ item: Deal }>({ + method: 'crm.item.get', + params: { entityTypeId: 2, id: 999_999 } + }) + if (!res.isSuccess) { + // Soft errors only (see softErrorCodes below). Most failures throw. + logger.warning('non-success result', { errors: res.getErrorMessages() }) + return + } + return res.getData()!.result.item + } catch (e) { + if (e instanceof AjaxError) { + logger.error('REST error', { code: e.code, status: e.status, message: e.message, requestInfo: e.requestInfo }) + } else if (e instanceof SdkError) { + logger.error('SDK error', { code: e.code, message: e.message }) + } else { + throw e + } } } ``` @@ -248,6 +251,7 @@ await $b24.setRestrictionManagerParams({ For heavy long-running calls, also raise the axios timeout on the underlying HTTP client: ```ts +import { ApiVersion } from '@bitrix24/b24jssdk' const clientAxios = $b24.getHttpClient(ApiVersion.v2).ajaxClient clientAxios.defaults.timeout = 120_000 ``` diff --git a/skills/b24jssdk-filtering/SKILL.md b/skills/b24jssdk-filtering/SKILL.md index e8e2a263..fbba03d2 100644 --- a/skills/b24jssdk-filtering/SKILL.md +++ b/skills/b24jssdk-filtering/SKILL.md @@ -12,11 +12,13 @@ Bitrix24 has **two filter dialects**. They are not interchangeable — each API Used by `$b24.actions.v2.{call,callList,fetchList}.make({ params: { filter: ... }})`. The operator is a prefix on the field name. ```ts -filter: { - '>=opportunity': 50000, - '<=opportunity': 200000, - '!stageId': 'LOST', - '=%title': 'A%' +const params = { + filter: { + '>=opportunity': 50000, + '<=opportunity': 200000, + '!stageId': 'LOST', + '=%title': 'A%' + } } ``` @@ -42,13 +44,13 @@ Multiple keys are combined with AND. Two operators on the same field need two se Plain array means `IN`: ```ts -filter: { stageId: ['NEW', 'PREPARATION', 'EXECUTING'] } +const params = { filter: { stageId: ['NEW', 'PREPARATION', 'EXECUTING'] } } ``` For "not in": ```ts -filter: { '!stageId': ['LOST', 'WON'] } +const params = { filter: { '!stageId': ['LOST', 'WON'] } } ``` ## v3 — array of triples @@ -79,8 +81,8 @@ filter: [ The two-arg form is sugar: ```ts -['id', 42] // same as ['id', '=', 42] -['stageId', ['A', 'B']] // same as ['stageId', 'in', ['A', 'B']] +const equals = ['id', 42] // same as ['id', '=', 42] +const oneOf = ['stageId', ['A', 'B']] // same as ['stageId', 'in', ['A', 'B']] ``` The long struct form supports nested groups with `or` logic and negation (rarely needed in user code): @@ -107,6 +109,7 @@ The top-level array is implicitly `logic: 'and'`. The `type: 'filter'` key above For anything beyond a flat list of triples, prefer the `FilterV3` builder over hand-writing the structs — it validates operators and `in`/`between` shapes on the client (a typo fails fast instead of as a server `UNKNOWNFILTEROPERATOREXCEPTION`): ```ts +import { Text } from '@bitrix24/b24jssdk' import { FilterV3 as F } from '@bitrix24/b24jssdk' // status = NEW AND (id in [1,2] OR id > 100) @@ -148,10 +151,10 @@ const sixMonthsAgo = new Date() sixMonthsAgo.setMonth(sixMonthsAgo.getMonth() - 6) // v2 -filter: { '>=createdTime': Text.toB24Format(sixMonthsAgo) } +const paramsV2 = { filter: { '>=createdTime': Text.toB24Format(sixMonthsAgo) } } // v3 -filter: [['createdTime', '>=', Text.toB24Format(sixMonthsAgo)]] +const paramsV3 = { filter: [['createdTime', '>=', Text.toB24Format(sixMonthsAgo)]] } ``` ## Field naming — v2 (classic vs v3-style) @@ -192,6 +195,7 @@ select: ['id', 'title', 'responsible.name', 'responsible.email'] ### v2 — open deals with amount range ```ts +import { EnumCrmEntityTypeId } from '@bitrix24/b24jssdk' const response = await $b24.actions.v2.callList.make({ method: 'crm.item.list', params: { @@ -211,6 +215,7 @@ const response = await $b24.actions.v2.callList.make({ ### v2 — contacts by phone substring ```ts +import { EnumCrmEntityTypeId } from '@bitrix24/b24jssdk' const response = await $b24.actions.v2.callList.make({ method: 'crm.item.list', params: { @@ -241,6 +246,7 @@ const response = await $b24.actions.v2.callList.make({ ### v3 — eventlog last 6 months ```ts +import { Text } from '@bitrix24/b24jssdk' const sixMonthsAgo = new Date() sixMonthsAgo.setMonth(sixMonthsAgo.getMonth() - 6) sixMonthsAgo.setHours(0, 0, 0, 0) @@ -260,6 +266,7 @@ const response = await $b24.actions.v3.callList.make({ ### v3 — IN / between ```ts +import { Text } from '@bitrix24/b24jssdk' filter: [ ['responsibleId', 'in', [1, 2, 3]], ['createdTime', 'between', ['2026-01-01T00:00:00+03:00', '2026-03-31T23:59:59+03:00']] diff --git a/skills/b24jssdk-frame-ui/SKILL.md b/skills/b24jssdk-frame-ui/SKILL.md index 7354d7f7..85a22bdf 100644 --- a/skills/b24jssdk-frame-ui/SKILL.md +++ b/skills/b24jssdk-frame-ui/SKILL.md @@ -104,7 +104,8 @@ const contact = single.contact?.[0] const picked: SelectedCRM = await $b24.dialog.selectCRM({ entityType: ['deal', 'company'], multiple: true, - value: { deal: ['D_42'], company: ['CO_7'] } + // Numeric ids, not the prefixed `D_42` / `CO_7` form the dialog returns. + value: { deal: [42], company: [7] } }) for (const deal of picked.deal ?? []) { @@ -123,7 +124,8 @@ The id format is per entity: ## Dialog — pick access targets (`selectAccess`) ```ts -const access = await $b24.dialog.selectAccess({ /* params */ }) +// The argument is a string[] of permission ids to block, not an options object. +const access = await $b24.dialog.selectAccess([]) ``` Less commonly used. Returns the parent window's raw access-selection payload — refer to Bitrix24's selectAccess docs for the shape. @@ -273,6 +275,7 @@ therefore always finish the install flow. ```ts +import { initializeB24Frame } from '@bitrix24/b24jssdk' const $b24 = await initializeB24Frame() if ($b24.isInstallMode) { diff --git a/skills/b24jssdk-helpers/SKILL.md b/skills/b24jssdk-helpers/SKILL.md index 87d6b573..9c5e936e 100644 --- a/skills/b24jssdk-helpers/SKILL.md +++ b/skills/b24jssdk-helpers/SKILL.md @@ -57,8 +57,8 @@ const me = helper.profileInfo.data const app = helper.appInfo.data const status = helper.appInfo.statusCode // EnumAppStatus -const license = helper.license // for enterprise checks -const payment = helper.payment +const license = helper.licenseInfo // for enterprise checks +const payment = helper.paymentInfo const baseCurrency = helper.currency.baseCurrency ``` @@ -92,7 +92,9 @@ await helper.appOptions.save( } ) -const flags = helper.appOptions.getJsonObject<{ exportCsv: boolean; beta: boolean }>( +// `getJsonObject` returns `object` — it takes no type argument. Cast at the +// callsite if you want a shape. +const flags = helper.appOptions.getJsonObject( 'featureFlags', { exportCsv: false, beta: false } ) @@ -103,6 +105,7 @@ const flags = helper.appOptions.getJsonObject<{ exportCsv: boolean; beta: boolea ## Pull client (push events) ```ts +import type { TypePullMessage } from '@bitrix24/b24jssdk' usePullClient() useSubscribePullClient((m: TypePullMessage) => { @@ -127,6 +130,9 @@ useSubscribePullClient(handleCrmEvent, 'crm') To shut down (also tears the Pull client down): ```ts +import { useB24Helper } from '@bitrix24/b24jssdk' + +const { destroyB24Helper } = useB24Helper() destroyB24Helper() ``` diff --git a/skills/b24jssdk-recipes/SKILL.md b/skills/b24jssdk-recipes/SKILL.md index 6ad0a5ef..37c20c9e 100644 --- a/skills/b24jssdk-recipes/SKILL.md +++ b/skills/b24jssdk-recipes/SKILL.md @@ -41,8 +41,14 @@ hiding it behind an import would cost more than the duplication does. Recipes 06 and 12 end with a guard instead of a bare `main()` call: ```ts +import { pathToFileURL } from 'node:url' + if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { - main().catch(...) + main().catch((e: unknown) => { + // Raw console.error, so structured-logger formatting cannot hide the trace. + console.error('\n[recipe failed]', e instanceof Error ? `${e.name}: ${e.message}` : String(e)) + process.exitCode = 1 + }) } ``` diff --git a/skills/b24jssdk-rest/SKILL.md b/skills/b24jssdk-rest/SKILL.md index c7cb6016..59939f57 100644 --- a/skills/b24jssdk-rest/SKILL.md +++ b/skills/b24jssdk-rest/SKILL.md @@ -77,6 +77,7 @@ const task = response.getData()!.result.task ## `batch.make` — array form ```ts +import { EnumCrmEntityTypeId } from '@bitrix24/b24jssdk' import type { AjaxResult } from '@bitrix24/b24jssdk' interface Contact { id: number; name: string } @@ -105,6 +106,7 @@ for (const r of results) { ## `batch.make` — named object form ```ts +import type { AjaxResult } from '@bitrix24/b24jssdk' interface Contact { id: number; name: string } interface Deal { id: number; title: string } @@ -128,6 +130,7 @@ Set `isHaltOnError: false` to collect per-command failures. **v3 batch is all-or To feed one v3 command's output into a later one, give it an `as` alias and reference it with the `BatchRefV3` markers (`import { BatchRefV3 } from '@bitrix24/b24jssdk'`): `BatchRefV3.ref('alias.item.id')` (single value) or `BatchRefV3.refArray('alias.id')` (a field collected across the alias's `items[]`). The server does the substitution. ```ts +import type { AjaxResult } from '@bitrix24/b24jssdk' const response = await $b24.actions.v2.batch.make<{ item: Contact }>({ calls: arrayOfCalls, options: { isHaltOnError: false, returnAjaxResult: true } @@ -145,6 +148,7 @@ For **object / named-command** calls (`calls: { name: { method, params } }`), th Chunk size is 50 per Bitrix24 batch limit. The action splits and re-aggregates: ```ts +import { EnumCrmEntityTypeId } from '@bitrix24/b24jssdk' import type { BatchCommandsArrayUniversal } from '@bitrix24/b24jssdk' const calls: BatchCommandsArrayUniversal = ids.map((id) => @@ -167,6 +171,7 @@ const items = data.map((row) => row.item) Loads up to 1000 items into a single array. Internally pages with a keyset cursor on `cursorIdKey` (which defaults to `idKey`). ```ts +import { EnumCrmEntityTypeId } from '@bitrix24/b24jssdk' import { Text } from '@bitrix24/b24jssdk' interface CrmItem { id: number; title: string } @@ -201,6 +206,7 @@ const items = response.getData()! // CrmItem[] Async iterator that yields chunks. Same shape as `callList.make` plus an optional `limit` for v3. ```ts +import { EnumCrmEntityTypeId } from '@bitrix24/b24jssdk' const generator = $b24.actions.v2.fetchList.make({ method: 'crm.item.list', params: { @@ -220,6 +226,7 @@ for await (const chunk of generator) { For v3: ```ts +import { Text } from '@bitrix24/b24jssdk' const generator = $b24.actions.v3.fetchList.make({ method: 'main.eventlog.list', params: { @@ -279,6 +286,7 @@ Removed from the public surface for `3.0.0`: A per-command `result` inside a batch can legitimately be `null` (e.g. `im.chat.get` with non-matching params — see issue #23). Type the generic as `T | null` and handle the null branch — the SDK no longer coerces to `{}`. ```ts +import type { AjaxResult } from '@bitrix24/b24jssdk' const response = await $b24.actions.v2.batch.make<{ result: ChatInfo | null }>({ calls: { Chat: ['im.chat.get', { chat_id: 999999 }] }, options: { returnAjaxResult: true } @@ -295,26 +303,28 @@ if (chat === null) { ```ts import { AjaxError, SdkError } from '@bitrix24/b24jssdk' -try { - const res = await $b24.actions.v2.call.make({ - method: 'crm.item.get', - params: { entityTypeId: 2, id: 999_999 } - }) - if (!res.isSuccess) { - // Soft errors (rare; usually you'll see throws) - logger.warning('non-success', { errors: res.getErrorMessages() }) - return - } - return res.getData()!.result.item -} catch (e) { - if (e instanceof AjaxError) { - // Bitrix24 REST error - logger.error('REST error', { code: e.code, status: e.status, message: e.message, requestInfo: e.requestInfo }) - } else if (e instanceof SdkError) { - // SDK-level error (wrong API version, etc.) - logger.error('SDK error', { code: e.code, message: e.message }) - } else { - throw e +async function loadDeal() { + try { + const res = await $b24.actions.v2.call.make<{ item: Deal }>({ + method: 'crm.item.get', + params: { entityTypeId: 2, id: 999_999 } + }) + if (!res.isSuccess) { + // Soft errors (rare; usually you'll see throws) + logger.warning('non-success', { errors: res.getErrorMessages() }) + return + } + return res.getData()!.result.item + } catch (e) { + if (e instanceof AjaxError) { + // Bitrix24 REST error + logger.error('REST error', { code: e.code, status: e.status, message: e.message, requestInfo: e.requestInfo }) + } else if (e instanceof SdkError) { + // SDK-level error (wrong API version, etc.) + logger.error('SDK error', { code: e.code, message: e.message }) + } else { + throw e + } } } ``` From ca4b08157feae311b5a767083e07efe9eeddb892 Mon Sep 17 00:00:00 2001 From: Shevchik Igor Date: Thu, 27 Aug 2026 09:23:39 +0000 Subject: [PATCH 2/3] fix(scripts): docs-lint read only committed history, so it disagreed with CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Twice now `docs-lint --strict` passed locally and then failed in CI on the same change. The mechanism: the freshness check asks `git log -1` for a cited source's last modification, so a source that is modified but not yet committed still reports its OLD date — the local run reports on a state that no longer exists, and the warning appears the moment the commit lands. The second occurrence came immediately after writing "re-run it AFTER committing" into a commit message, which is the evidence that the rule does not work as a rule. It is now unnecessary: an uncommitted edit to a cited source counts as modified now, so the local run agrees with CI whatever the commit state. Verified both directions — dirtying a cited source produces the warning with today's date, restoring it clears it — and the regression test drives the real `gitLastCommitDate` rather than the injected seam, so the git plumbing is what is under test. Removing the new branch fails it. Also refreshes the `audited:` stamps on the two pages citing `index.ts`, which exporting `B24HelperManager` invalidated. Checked, not rubber-stamped: the API reference page gained its row in the same change, and the types index lists types rather than value exports, so a new class does not alter what it claims. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F22e2ft66y7nuBJjzdThBr --- .../90.types-index.md | 2 +- docs/content/docs/3.api-reference/1.index.md | 2 +- scripts/__tests__/docs-lint.test.mjs | 37 ++++++++++++++++++- scripts/docs-lint.mjs | 19 ++++++++++ 4 files changed, 57 insertions(+), 3 deletions(-) diff --git a/docs/content/docs/2.working-with-the-rest-api/90.types-index.md b/docs/content/docs/2.working-with-the-rest-api/90.types-index.md index e9bd7854..96998a7e 100644 --- a/docs/content/docs/2.working-with-the-rest-api/90.types-index.md +++ b/docs/content/docs/2.working-with-the-rest-api/90.types-index.md @@ -3,7 +3,7 @@ title: Types overview description: 'Navigable map of every types/* module in the SDK — which have a dedicated page, which are surfaced through another page, and which are follow-ups.' category: 'Types' navigation.title: Types overview -audited: 2026-07-02 +audited: 2026-08-27 links: - label: public barrel (index.ts) iconName: GitHubIcon diff --git a/docs/content/docs/3.api-reference/1.index.md b/docs/content/docs/3.api-reference/1.index.md index a01255f1..d37969b3 100644 --- a/docs/content/docs/3.api-reference/1.index.md +++ b/docs/content/docs/3.api-reference/1.index.md @@ -2,7 +2,7 @@ title: API Reference description: 'Index of the public @bitrix24/b24jssdk surface, grouped by domain — every value export, what it is, and where its guide and source live.' navigation.title: API Reference -audited: 2026-08-24 +audited: 2026-08-27 links: - label: index.ts (the public surface) iconName: GitHubIcon diff --git a/scripts/__tests__/docs-lint.test.mjs b/scripts/__tests__/docs-lint.test.mjs index 97d89b13..769a83cc 100644 --- a/scripts/__tests__/docs-lint.test.mjs +++ b/scripts/__tests__/docs-lint.test.mjs @@ -8,7 +8,7 @@ // // Run with: node --test scripts/__tests__/docs-lint.test.mjs -import { mkdtempSync, mkdirSync, writeFileSync, rmSync, symlinkSync } from 'node:fs' +import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync, symlinkSync } from 'node:fs' import { tmpdir } from 'node:os' import { join, resolve, dirname } from 'node:path' import { fileURLToPath } from 'node:url' @@ -154,6 +154,41 @@ test('checkAuditFreshness: a .md source never ages a page; a .ts source does', ( assert.doesNotMatch(warns[0], /SKILL\.md/) }) +test('gitLastCommitDate: an uncommitted edit to a cited source ages the page', () => { + // The trap this closes, twice hit in practice: the freshness check read only + // committed history, so running it with a cited source modified-but-not-yet- + // committed reported on a state that no longer existed — clean locally, red in + // CI the moment the commit landed. + // + // Driven through the real `gitLastCommitDate` (no `getCommitDate` seam) so the + // git plumbing itself is exercised: dirty the file, expect a warning; restore + // it, expect none. + const cited = 'packages/jssdk/src/types/payloads.ts' + const abs = join(REPO_ROOT, cited) + const original = readFileSync(abs, 'utf8') + const frontmatter = { + audited: '2020-01-01', + links: [`label: Code\nto: https://github.com/bitrix24/b24jssdk/blob/main/${cited}`] + } + + const cleanWarns = [] + checkAuditFreshness('page.md', frontmatter, { warn: (_f, m) => cleanWarns.push(m) }) + // The audited date is deliberately ancient, so a committed source warns too — + // that is the baseline the dirty case has to beat, not a bug. + assert.equal(cleanWarns.length, 1) + + try { + writeFileSync(abs, original + '\n// probe\n', 'utf8') + const dirtyWarns = [] + checkAuditFreshness('page.md', frontmatter, { warn: (_f, m) => dirtyWarns.push(m) }) + assert.equal(dirtyWarns.length, 1) + // Today's date, not the last commit's — that is the whole point. + assert.match(dirtyWarns[0], new RegExp(`modified on ${new Date().toISOString().slice(0, 10)}`)) + } finally { + writeFileSync(abs, original, 'utf8') + } +}) + // ── checkFrontmatterLinkTargets (#117) ─────────────────────────────────── test('checkFrontmatterLinkTargets: errors on a blob/main link whose file is gone', () => { diff --git a/scripts/docs-lint.mjs b/scripts/docs-lint.mjs index 49aefddd..fcb52167 100644 --- a/scripts/docs-lint.mjs +++ b/scripts/docs-lint.mjs @@ -78,6 +78,25 @@ function gitLastCommitDate(localPath) { ['log', '-1', '--format=%cI', '--', localPath], { cwd: REPO_ROOT, encoding: 'utf8' } ).trim() + + // An uncommitted edit counts as "modified now". + // + // Without this the check reads only committed history, so running it with a + // cited source modified-but-not-yet-committed reports on a state that no + // longer exists — it passes locally and then fails in CI the moment the + // commit lands. That happened twice in a row, the second time immediately + // after the lesson was written down, which is the evidence that "remember to + // run it after committing" is not a workable rule. Reading the working tree + // makes the local run agree with CI whatever the commit state. + const dirty = execFileSync( + 'git', + ['status', '--porcelain', '--', localPath], + { cwd: REPO_ROOT, encoding: 'utf8' } + ).trim() + if (dirty) { + return new Date().toISOString() + } + return out || null } catch { return null From 409efdf2f67b6e1c5b41f82e6c1da0101b1b4006 Mon Sep 17 00:00:00 2001 From: Shevchik Igor Date: Thu, 27 Aug 2026 09:46:01 +0000 Subject: [PATCH 3/3] fix(scripts): test the freshness branch that actually matters, and stop spawning git per link MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found three things, and my own mutation testing found a fourth that mattered more than any of them. One `git status` for the run, not one per cited link. There are ~84 audited links; a spawn each doubled the script's process count for information a single call already holds. Parsing is now a pure exported function, so the rename and quoted-path cases are tested directly rather than by making the repository actually contain them. A gitignored cited source is no longer skipped in silence. `git status --porcelain` does not list an ignored path and `git log` has no commit for it, so both signals were blind and the page's audit was never checked at all — the least useful of the three possible outcomes. It now counts as modified. The test wrote to a real tracked source. It appended a probe line to `packages/jssdk/src/types/payloads.ts` and restored it in a `finally`, which loses the race against a killed runner and leaves a corrupted source file. Rewritten to use a throwaway path, now also gitignored so a killed run leaves nothing behind at all. And the one I would have shipped: the dirty-TRACKED-file branch — the whole point of the change — had NO test. Both probe tests exit through the untracked/ignored fallback, so disabling that branch left the suite green. It is now covered at the function that owns it, with `isDirty` injected rather than by dirtying a real source. All three branches fail under mutation; before this, one of them failed under none. The earlier version of this test was also wrong in a way that passed review: it set `audited` in the FUTURE so that committed history would look fresh, but "modified now" cannot exceed a future date either, so the assertion could not have fired for the right reason. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01F22e2ft66y7nuBJjzdThBr --- .gitignore | 4 + scripts/__tests__/docs-lint.test.mjs | 105 +++++++++++++++++++++++---- scripts/docs-lint.mjs | 64 +++++++++++++--- 3 files changed, 148 insertions(+), 25 deletions(-) diff --git a/.gitignore b/.gitignore index ad64abda..52fa0819 100644 --- a/.gitignore +++ b/.gitignore @@ -111,4 +111,8 @@ coverage # docs-typecheck generated temp files (scripts/docs-typecheck.mjs) .docs-typecheck/tmp/ .skills-typecheck/tmp/ +# Scratch file written by the docs-lint freshness test. It is removed in a +# `finally`, but a killed runner would otherwise leave it as a stray untracked +# file — ignoring it keeps `git status` honest either way. +packages/jssdk/src/.docs-lint-freshness-probe.ts __pycache__/ diff --git a/scripts/__tests__/docs-lint.test.mjs b/scripts/__tests__/docs-lint.test.mjs index 769a83cc..29e42227 100644 --- a/scripts/__tests__/docs-lint.test.mjs +++ b/scripts/__tests__/docs-lint.test.mjs @@ -8,7 +8,7 @@ // // Run with: node --test scripts/__tests__/docs-lint.test.mjs -import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, rmSync, symlinkSync } from 'node:fs' +import { mkdtempSync, mkdirSync, writeFileSync, rmSync, symlinkSync } from 'node:fs' import { tmpdir } from 'node:os' import { join, resolve, dirname } from 'node:path' import { fileURLToPath } from 'node:url' @@ -16,7 +16,7 @@ import { spawnSync } from 'node:child_process' import test from 'node:test' import assert from 'node:assert/strict' import { parseFrontmatter, walkMarkdownFiles, isFreshnessTrackedSource } from '../_docs-utils.mjs' -import { checkAuditFreshness, checkFrontmatterLinkTargets } from '../docs-lint.mjs' +import { checkAuditFreshness, checkFrontmatterLinkTargets, parseDirtyPaths, gitLastCommitDate } from '../docs-lint.mjs' const __dirname = dirname(fileURLToPath(import.meta.url)) const REPO_ROOT = resolve(__dirname, '..', '..') @@ -163,29 +163,102 @@ test('gitLastCommitDate: an uncommitted edit to a cited source ages the page', ( // Driven through the real `gitLastCommitDate` (no `getCommitDate` seam) so the // git plumbing itself is exercised: dirty the file, expect a warning; restore // it, expect none. - const cited = 'packages/jssdk/src/types/payloads.ts' + // A throwaway file inside the repo, never a tracked source. An earlier draft + // appended a probe line to `packages/jssdk/src/types/payloads.ts` and restored + // it in a `finally` — which loses the race if the runner is killed or times + // out, leaving a real source file corrupted. `git status` needs the path to be + // inside the work tree, but it does not need it to be one that matters. + const cited = 'packages/jssdk/src/.docs-lint-freshness-probe.ts' const abs = join(REPO_ROOT, cited) - const original = readFileSync(abs, 'utf8') const frontmatter = { audited: '2020-01-01', links: [`label: Code\nto: https://github.com/bitrix24/b24jssdk/blob/main/${cited}`] } - const cleanWarns = [] - checkAuditFreshness('page.md', frontmatter, { warn: (_f, m) => cleanWarns.push(m) }) - // The audited date is deliberately ancient, so a committed source warns too — - // that is the baseline the dirty case has to beat, not a bug. - assert.equal(cleanWarns.length, 1) + try { + writeFileSync(abs, '// probe\n', 'utf8') + const warns = [] + checkAuditFreshness('page.md', frontmatter, { warn: (_f, m) => warns.push(m) }) + + assert.equal(warns.length, 1, 'an untracked/dirty cited source must age the page') + // Today's date, not a commit's — that is the whole point. The file has no + // commit at all, so reading history alone would have skipped it silently. + assert.match(warns[0], new RegExp(`modified on ${new Date().toISOString().slice(0, 10)}`)) + } finally { + rmSync(abs, { force: true }) + } +}) + +test('parseDirtyPaths: reads every shape git status --porcelain emits', () => { + const paths = parseDirtyPaths([ + ' M packages/jssdk/src/types/http.ts', // modified, unstaged + 'M packages/jssdk/src/index.ts', // staged + 'MM scripts/docs-lint.mjs', // staged and modified again + '?? scripts/new-thing.mjs', // untracked + 'A docs/content/docs/new-page.md', // added + 'R old/name.ts -> packages/jssdk/src/renamed.ts', // rename: destination wins + '?? "docs/content/a file with spaces.md"' // quoted + ].join('\n')) + + assert.ok(paths.has('packages/jssdk/src/types/http.ts')) + assert.ok(paths.has('packages/jssdk/src/index.ts')) + assert.ok(paths.has('scripts/docs-lint.mjs')) + assert.ok(paths.has('scripts/new-thing.mjs')) + assert.ok(paths.has('docs/content/docs/new-page.md')) + // The rename records where the file IS, not where it was. + assert.ok(paths.has('packages/jssdk/src/renamed.ts')) + assert.ok(!paths.has('old/name.ts')) + assert.ok(paths.has('docs/content/a file with spaces.md')) + assert.equal(paths.size, 7) +}) + +test('gitLastCommitDate: a dirty TRACKED source reports now, not its commit date', () => { + // The primary case, and the one neither probe-file test reaches: both exit + // through the untracked/ignored fallback, so disabling the dirty branch left + // them green. Tested at the function that owns the branch, with `isDirty` + // injected — the alternative, dirtying a real tracked source, is the exact + // hazard the probe tests were rewritten to avoid. + const cited = 'packages/jssdk/src/core/result.ts' + const today = new Date().toISOString().slice(0, 10) + + const clean = gitLastCommitDate(cited, () => false) + assert.ok(clean, 'a tracked file must have a commit date') + assert.notEqual(clean.slice(0, 10), today, 'fixture assumption: result.ts was not committed today') + + const dirty = gitLastCommitDate(cited, () => true) + assert.equal(dirty.slice(0, 10), today, 'a dirty source must report as modified now') +}) + +test('gitLastCommitDate: a gitignored cited source ages the page too', () => { + // The narrower half of the same trap, and the one the test above does NOT + // reach: an untracked file shows up in `git status` as `??`, so it exits + // through the dirty branch. A GITIGNORED file appears in neither `git status` + // nor `git log` — so before this it returned null and the page was skipped in + // silence, which is the worst of the three outcomes. + // + // `.docs-typecheck/tmp/` is ignored by .gitignore:112, which is what makes it + // usable here. + const dir = join(REPO_ROOT, '.docs-typecheck', 'tmp') + const cited = '.docs-typecheck/tmp/freshness-probe.ts' + const abs = join(REPO_ROOT, cited) + mkdirSync(dir, { recursive: true }) try { - writeFileSync(abs, original + '\n// probe\n', 'utf8') - const dirtyWarns = [] - checkAuditFreshness('page.md', frontmatter, { warn: (_f, m) => dirtyWarns.push(m) }) - assert.equal(dirtyWarns.length, 1) - // Today's date, not the last commit's — that is the whole point. - assert.match(dirtyWarns[0], new RegExp(`modified on ${new Date().toISOString().slice(0, 10)}`)) + writeFileSync(abs, '// probe\n', 'utf8') + // Precondition, asserted rather than assumed: git must be blind to it both ways. + assert.equal(spawnSync('git', ['status', '--porcelain', '--', cited], { cwd: REPO_ROOT, encoding: 'utf8' }).stdout.trim(), '') + assert.equal(spawnSync('git', ['log', '-1', '--format=%cI', '--', cited], { cwd: REPO_ROOT, encoding: 'utf8' }).stdout.trim(), '') + + const warns = [] + checkAuditFreshness('page.md', { + audited: '2020-01-01', + links: [`label: Code\nto: https://github.com/bitrix24/b24jssdk/blob/main/${cited}`] + }, { warn: (_f, m) => warns.push(m) }) + + assert.equal(warns.length, 1, 'a cited source git cannot vouch for must age the page') + assert.match(warns[0], new RegExp(`modified on ${new Date().toISOString().slice(0, 10)}`)) } finally { - writeFileSync(abs, original, 'utf8') + rmSync(abs, { force: true }) } }) diff --git a/scripts/docs-lint.mjs b/scripts/docs-lint.mjs index fcb52167..4ec9c6c6 100644 --- a/scripts/docs-lint.mjs +++ b/scripts/docs-lint.mjs @@ -62,7 +62,53 @@ function extractGithubLinkPaths(arrayItems) { return paths } -function gitLastCommitDate(localPath) { +/** + * Paths with uncommitted changes, as one lookup for the whole run. + * + * One `git status` for the repository instead of one per cited link: there are + * ~84 audited links, and a spawn each doubled the process count of the script + * for information a single call already contains. + * + * Built lazily, so runs that never reach a freshness check pay nothing. + */ +/** + * Parse `git status --porcelain` into the set of paths it reports. + * + * Exported and pure so the parsing is testable without dirtying a real file: + * proving the rename and quoting cases by making the repository actually + * contain them would be far more hazardous than the bug it guards. + * + * Each line is `XY path`, or `XY old -> new` for a rename — the destination is + * the path that now exists. Paths containing a space or a non-ASCII byte come + * back quoted. + */ +export function parseDirtyPaths(porcelain) { + const paths = new Set() + for (const line of porcelain.split('\n')) { + if (!line.trim()) continue + const parts = line.slice(3).trim().split(' -> ') + paths.add(parts[parts.length - 1].replace(/^"|"$/g, '')) + } + return paths +} + +let dirtyPaths = null +function isDirty(localPath) { + if (dirtyPaths === null) { + try { + dirtyPaths = parseDirtyPaths( + execFileSync('git', ['status', '--porcelain'], { cwd: REPO_ROOT, encoding: 'utf8' }) + ) + } catch { + // No git, or not a repository. Freshness then rests on `git log` alone, + // which is what this check did before. + dirtyPaths = new Set() + } + } + return dirtyPaths.has(localPath) +} + +export function gitLastCommitDate(localPath, isDirtyImpl = isDirty) { const abs = join(REPO_ROOT, localPath) try { statSync(abs) @@ -88,16 +134,16 @@ function gitLastCommitDate(localPath) { // after the lesson was written down, which is the evidence that "remember to // run it after committing" is not a workable rule. Reading the working tree // makes the local run agree with CI whatever the commit state. - const dirty = execFileSync( - 'git', - ['status', '--porcelain', '--', localPath], - { cwd: REPO_ROOT, encoding: 'utf8' } - ).trim() - if (dirty) { + if (isDirtyImpl(localPath)) { return new Date().toISOString() } - return out || null + // Exists on disk, yet git knows no commit for it — it is gitignored, and + // `git status --porcelain` does not list an ignored path either. Both of + // this function's signals are blind to it, so the audit cannot be vouched + // for against history at all: warn rather than skip the page in silence, + // which was the previous behaviour and the least useful of the three. + return out || new Date().toISOString() } catch { return null } @@ -144,7 +190,7 @@ function checkActionSkeleton(file, body) { export function checkAuditFreshness(file, frontmatter, deps = {}) { if (!frontmatter.audited) return // Dependency seams so tests can exercise the skip logic without a git history. - const getCommitDate = deps.getCommitDate || gitLastCommitDate + const getCommitDate = deps.getCommitDate || (path => gitLastCommitDate(path, deps.isDirty)) const warn = deps.warn || ((f, m) => log('warn', f, m)) const auditedDate = new Date(frontmatter.audited + 'T23:59:59Z') const ghPaths = extractGithubLinkPaths(frontmatter.links || [])