From dbc78243f40550df85cd4be83e464da5c63b4416 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 2 Mar 2026 14:36:00 +0000 Subject: [PATCH] refactor(scripts): share guard runners and paged select UI --- .pi/extensions/diff.ts | 100 ++--------------- .pi/extensions/files.ts | 104 ++++-------------- .pi/extensions/ui/paged-select.ts | 82 ++++++++++++++ scripts/check-no-pairing-store-group-auth.mjs | 14 +-- scripts/check-no-random-messaging-tmp.mjs | 68 +++--------- scripts/check-no-raw-channel-fetch.mjs | 78 ++++--------- scripts/check-pairing-account-scope.mjs | 6 +- scripts/lib/callsite-guard.mjs | 45 ++++++++ scripts/lib/pairing-guard-context.mjs | 13 +++ scripts/lib/ts-guard-utils.mjs | 18 ++- 10 files changed, 234 insertions(+), 294 deletions(-) create mode 100644 .pi/extensions/ui/paged-select.ts create mode 100644 scripts/lib/callsite-guard.mjs create mode 100644 scripts/lib/pairing-guard-context.mjs diff --git a/.pi/extensions/diff.ts b/.pi/extensions/diff.ts index 037fa240afb..9f8e718e892 100644 --- a/.pi/extensions/diff.ts +++ b/.pi/extensions/diff.ts @@ -6,15 +6,7 @@ */ import type { ExtensionAPI } from "@mariozechner/pi-coding-agent"; -import { DynamicBorder } from "@mariozechner/pi-coding-agent"; -import { - Container, - Key, - matchesKey, - type SelectItem, - SelectList, - Text, -} from "@mariozechner/pi-tui"; +import { showPagedSelectList } from "./ui/paged-select"; interface FileInfo { status: string; @@ -108,87 +100,17 @@ export default function (pi: ExtensionAPI) { } }; - // Show file picker with SelectList - await ctx.ui.custom((tui, theme, _kb, done) => { - const container = new Container(); - - // Top border - container.addChild(new DynamicBorder((s: string) => theme.fg("accent", s))); - - // Title - container.addChild(new Text(theme.fg("accent", theme.bold(" Select file to diff")), 0, 0)); - - // Build select items with colored status - const items: SelectItem[] = files.map((f) => { - let statusColor: string; - switch (f.status) { - case "M": - statusColor = theme.fg("warning", f.status); - break; - case "A": - statusColor = theme.fg("success", f.status); - break; - case "D": - statusColor = theme.fg("error", f.status); - break; - case "?": - statusColor = theme.fg("muted", f.status); - break; - default: - statusColor = theme.fg("dim", f.status); - } - return { - value: f, - label: `${statusColor} ${f.file}`, - }; - }); - - const visibleRows = Math.min(files.length, 15); - let currentIndex = 0; - - const selectList = new SelectList(items, visibleRows, { - selectedPrefix: (t) => theme.fg("accent", t), - selectedText: (t) => t, // Keep existing colors - description: (t) => theme.fg("muted", t), - scrollInfo: (t) => theme.fg("dim", t), - noMatch: (t) => theme.fg("warning", t), - }); - selectList.onSelect = (item) => { + const items = files.map((file) => ({ + value: file, + label: `${file.status} ${file.file}`, + })); + await showPagedSelectList({ + ctx, + title: " Select file to diff", + items, + onSelect: (item) => { void openSelected(item.value as FileInfo); - }; - selectList.onCancel = () => done(); - selectList.onSelectionChange = (item) => { - currentIndex = items.indexOf(item); - }; - container.addChild(selectList); - - // Help text - container.addChild( - new Text(theme.fg("dim", " ↑↓ navigate • ←→ page • enter open • esc close"), 0, 0), - ); - - // Bottom border - container.addChild(new DynamicBorder((s: string) => theme.fg("accent", s))); - - return { - render: (w) => container.render(w), - invalidate: () => container.invalidate(), - handleInput: (data) => { - // Add paging with left/right - if (matchesKey(data, Key.left)) { - // Page up - clamp to 0 - currentIndex = Math.max(0, currentIndex - visibleRows); - selectList.setSelectedIndex(currentIndex); - } else if (matchesKey(data, Key.right)) { - // Page down - clamp to last - currentIndex = Math.min(items.length - 1, currentIndex + visibleRows); - selectList.setSelectedIndex(currentIndex); - } else { - selectList.handleInput(data); - } - tui.requestRender(); - }, - }; + }, }); }, }); diff --git a/.pi/extensions/files.ts b/.pi/extensions/files.ts index bba2760d032..e1325303521 100644 --- a/.pi/extensions/files.ts +++ b/.pi/extensions/files.ts @@ -6,15 +6,7 @@ */ import type { ExtensionAPI } from "@mariozechner/pi-coding-agent"; -import { DynamicBorder } from "@mariozechner/pi-coding-agent"; -import { - Container, - Key, - matchesKey, - type SelectItem, - SelectList, - Text, -} from "@mariozechner/pi-tui"; +import { showPagedSelectList } from "./ui/paged-select"; interface FileEntry { path: string; @@ -113,82 +105,30 @@ export default function (pi: ExtensionAPI) { } }; - // Show file picker with SelectList - await ctx.ui.custom((tui, theme, _kb, done) => { - const container = new Container(); - - // Top border - container.addChild(new DynamicBorder((s: string) => theme.fg("accent", s))); - - // Title - container.addChild(new Text(theme.fg("accent", theme.bold(" Select file to open")), 0, 0)); - - // Build select items with colored operations - const items: SelectItem[] = files.map((f) => { - const ops: string[] = []; - if (f.operations.has("read")) { - ops.push(theme.fg("muted", "R")); - } - if (f.operations.has("write")) { - ops.push(theme.fg("success", "W")); - } - if (f.operations.has("edit")) { - ops.push(theme.fg("warning", "E")); - } - const opsLabel = ops.join(""); - return { - value: f, - label: `${opsLabel} ${f.path}`, - }; - }); - - const visibleRows = Math.min(files.length, 15); - let currentIndex = 0; - - const selectList = new SelectList(items, visibleRows, { - selectedPrefix: (t) => theme.fg("accent", t), - selectedText: (t) => t, // Keep existing colors - description: (t) => theme.fg("muted", t), - scrollInfo: (t) => theme.fg("dim", t), - noMatch: (t) => theme.fg("warning", t), - }); - selectList.onSelect = (item) => { - void openSelected(item.value as FileEntry); - }; - selectList.onCancel = () => done(); - selectList.onSelectionChange = (item) => { - currentIndex = items.indexOf(item); - }; - container.addChild(selectList); - - // Help text - container.addChild( - new Text(theme.fg("dim", " ↑↓ navigate • ←→ page • enter open • esc close"), 0, 0), - ); - - // Bottom border - container.addChild(new DynamicBorder((s: string) => theme.fg("accent", s))); - + const items = files.map((file) => { + const ops: string[] = []; + if (file.operations.has("read")) { + ops.push("R"); + } + if (file.operations.has("write")) { + ops.push("W"); + } + if (file.operations.has("edit")) { + ops.push("E"); + } return { - render: (w) => container.render(w), - invalidate: () => container.invalidate(), - handleInput: (data) => { - // Add paging with left/right - if (matchesKey(data, Key.left)) { - // Page up - clamp to 0 - currentIndex = Math.max(0, currentIndex - visibleRows); - selectList.setSelectedIndex(currentIndex); - } else if (matchesKey(data, Key.right)) { - // Page down - clamp to last - currentIndex = Math.min(items.length - 1, currentIndex + visibleRows); - selectList.setSelectedIndex(currentIndex); - } else { - selectList.handleInput(data); - } - tui.requestRender(); - }, + value: file, + label: `${ops.join("")} ${file.path}`, }; }); + await showPagedSelectList({ + ctx, + title: " Select file to open", + items, + onSelect: (item) => { + void openSelected(item.value as FileEntry); + }, + }); }, }); } diff --git a/.pi/extensions/ui/paged-select.ts b/.pi/extensions/ui/paged-select.ts new file mode 100644 index 00000000000..a92db66bc68 --- /dev/null +++ b/.pi/extensions/ui/paged-select.ts @@ -0,0 +1,82 @@ +import { DynamicBorder } from "@mariozechner/pi-coding-agent"; +import { + Container, + Key, + matchesKey, + type SelectItem, + SelectList, + Text, +} from "@mariozechner/pi-tui"; + +type CustomUiContext = { + ui: { + custom: ( + render: ( + tui: { requestRender: () => void }, + theme: { + fg: (tone: string, text: string) => string; + bold: (text: string) => string; + }, + kb: unknown, + done: () => void, + ) => { + render: (width: number) => string; + invalidate: () => void; + handleInput: (data: string) => void; + }, + ) => Promise; + }; +}; + +export async function showPagedSelectList(params: { + ctx: CustomUiContext; + title: string; + items: SelectItem[]; + onSelect: (item: SelectItem) => void; +}): Promise { + await params.ctx.ui.custom((tui, theme, _kb, done) => { + const container = new Container(); + + container.addChild(new DynamicBorder((s: string) => theme.fg("accent", s))); + container.addChild(new Text(theme.fg("accent", theme.bold(params.title)), 0, 0)); + + const visibleRows = Math.min(params.items.length, 15); + let currentIndex = 0; + + const selectList = new SelectList(params.items, visibleRows, { + selectedPrefix: (text) => theme.fg("accent", text), + selectedText: (text) => text, + description: (text) => theme.fg("muted", text), + scrollInfo: (text) => theme.fg("dim", text), + noMatch: (text) => theme.fg("warning", text), + }); + selectList.onSelect = (item) => params.onSelect(item); + selectList.onCancel = () => done(); + selectList.onSelectionChange = (item) => { + currentIndex = params.items.indexOf(item); + }; + container.addChild(selectList); + + container.addChild( + new Text(theme.fg("dim", " ↑↓ navigate • ←→ page • enter open • esc close"), 0, 0), + ); + container.addChild(new DynamicBorder((s: string) => theme.fg("accent", s))); + + return { + render: (width) => container.render(width), + invalidate: () => container.invalidate(), + handleInput: (data) => { + if (matchesKey(data, Key.left)) { + currentIndex = Math.max(0, currentIndex - visibleRows); + selectList.setSelectedIndex(currentIndex); + } else if (matchesKey(data, Key.right)) { + currentIndex = Math.min(params.items.length - 1, currentIndex + visibleRows); + selectList.setSelectedIndex(currentIndex); + } else { + selectList.handleInput(data); + } + tui.requestRender(); + }, + }; + }); +} diff --git a/scripts/check-no-pairing-store-group-auth.mjs b/scripts/check-no-pairing-store-group-auth.mjs index 2ee94af82e1..83b3535abb3 100644 --- a/scripts/check-no-pairing-store-group-auth.mjs +++ b/scripts/check-no-pairing-store-group-auth.mjs @@ -1,24 +1,22 @@ #!/usr/bin/env node -import path from "node:path"; import ts from "typescript"; +import { createPairingGuardContext } from "./lib/pairing-guard-context.mjs"; import { collectFileViolations, getPropertyNameText, - resolveRepoRoot, runAsScript, toLine, } from "./lib/ts-guard-utils.mjs"; -const repoRoot = resolveRepoRoot(import.meta.url); -const sourceRoots = [path.join(repoRoot, "src"), path.join(repoRoot, "extensions")]; +const { repoRoot, sourceRoots, resolveFromRepo } = createPairingGuardContext(import.meta.url); const allowedFiles = new Set([ - path.join(repoRoot, "src", "security", "dm-policy-shared.ts"), - path.join(repoRoot, "src", "channels", "allow-from.ts"), + resolveFromRepo("src/security/dm-policy-shared.ts"), + resolveFromRepo("src/channels/allow-from.ts"), // Config migration/audit logic may intentionally reference store + group fields. - path.join(repoRoot, "src", "security", "fix.ts"), - path.join(repoRoot, "src", "security", "audit-channel.ts"), + resolveFromRepo("src/security/fix.ts"), + resolveFromRepo("src/security/audit-channel.ts"), ]); const storeIdentifierRe = /^(?:storeAllowFrom|storedAllowFrom|storeAllowList)$/i; diff --git a/scripts/check-no-random-messaging-tmp.mjs b/scripts/check-no-random-messaging-tmp.mjs index 170f9a3a994..ae5469d6deb 100644 --- a/scripts/check-no-random-messaging-tmp.mjs +++ b/scripts/check-no-random-messaging-tmp.mjs @@ -1,25 +1,17 @@ #!/usr/bin/env node -import { promises as fs } from "node:fs"; -import path from "node:path"; import ts from "typescript"; -import { - collectTypeScriptFiles, - resolveRepoRoot, - runAsScript, - toLine, - unwrapExpression, -} from "./lib/ts-guard-utils.mjs"; +import { runCallsiteGuard } from "./lib/callsite-guard.mjs"; +import { runAsScript, toLine, unwrapExpression } from "./lib/ts-guard-utils.mjs"; -const repoRoot = resolveRepoRoot(import.meta.url); const sourceRoots = [ - path.join(repoRoot, "src", "channels"), - path.join(repoRoot, "src", "infra", "outbound"), - path.join(repoRoot, "src", "line"), - path.join(repoRoot, "src", "media-understanding"), - path.join(repoRoot, "extensions"), + "src/channels", + "src/infra/outbound", + "src/line", + "src/media-understanding", + "extensions", ]; -const allowedCallsites = new Set([path.join(repoRoot, "extensions", "feishu", "src", "dedup.ts")]); +const allowedRelativePaths = new Set(["extensions/feishu/src/dedup.ts"]); function collectOsTmpdirImports(sourceFile) { const osModuleSpecifiers = new Set(["node:os", "os"]); @@ -82,40 +74,16 @@ export function findMessagingTmpdirCallLines(content, fileName = "source.ts") { } export async function main() { - const files = ( - await Promise.all( - sourceRoots.map( - async (dir) => - await collectTypeScriptFiles(dir, { - ignoreMissing: true, - }), - ), - ) - ).flat(); - const violations = []; - - for (const filePath of files) { - if (allowedCallsites.has(filePath)) { - continue; - } - const content = await fs.readFile(filePath, "utf8"); - for (const line of findMessagingTmpdirCallLines(content, filePath)) { - violations.push(`${path.relative(repoRoot, filePath)}:${line}`); - } - } - - if (violations.length === 0) { - return; - } - - console.error("Found os.tmpdir()/tmpdir() usage in messaging/channel runtime sources:"); - for (const violation of violations) { - console.error(`- ${violation}`); - } - console.error( - "Use resolvePreferredOpenClawTmpDir() or plugin-sdk temp helpers instead of host tmp defaults.", - ); - process.exit(1); + await runCallsiteGuard({ + importMetaUrl: import.meta.url, + sourceRoots, + findCallLines: findMessagingTmpdirCallLines, + skipRelativePath: (relativePath) => allowedRelativePaths.has(relativePath), + header: "Found os.tmpdir()/tmpdir() usage in messaging/channel runtime sources:", + footer: + "Use resolvePreferredOpenClawTmpDir() or plugin-sdk temp helpers instead of host tmp defaults.", + sortViolations: false, + }); } runAsScript(import.meta.url, main); diff --git a/scripts/check-no-raw-channel-fetch.mjs b/scripts/check-no-raw-channel-fetch.mjs index 616e9c23464..566034c6ca9 100644 --- a/scripts/check-no-raw-channel-fetch.mjs +++ b/scripts/check-no-raw-channel-fetch.mjs @@ -1,28 +1,20 @@ #!/usr/bin/env node -import { promises as fs } from "node:fs"; -import path from "node:path"; import ts from "typescript"; -import { - collectTypeScriptFiles, - resolveRepoRoot, - runAsScript, - toLine, - unwrapExpression, -} from "./lib/ts-guard-utils.mjs"; +import { runCallsiteGuard } from "./lib/callsite-guard.mjs"; +import { runAsScript, toLine, unwrapExpression } from "./lib/ts-guard-utils.mjs"; -const repoRoot = resolveRepoRoot(import.meta.url); const sourceRoots = [ - path.join(repoRoot, "src", "telegram"), - path.join(repoRoot, "src", "discord"), - path.join(repoRoot, "src", "slack"), - path.join(repoRoot, "src", "signal"), - path.join(repoRoot, "src", "imessage"), - path.join(repoRoot, "src", "web"), - path.join(repoRoot, "src", "channels"), - path.join(repoRoot, "src", "routing"), - path.join(repoRoot, "src", "line"), - path.join(repoRoot, "extensions"), + "src/telegram", + "src/discord", + "src/slack", + "src/signal", + "src/imessage", + "src/web", + "src/channels", + "src/routing", + "src/line", + "extensions", ]; // Temporary allowlist for legacy callsites. New raw fetch callsites in channel/plugin runtime @@ -100,43 +92,15 @@ export function findRawFetchCallLines(content, fileName = "source.ts") { } export async function main() { - const files = ( - await Promise.all( - sourceRoots.map( - async (sourceRoot) => - await collectTypeScriptFiles(sourceRoot, { - extraTestSuffixes: [".browser.test.ts", ".node.test.ts"], - ignoreMissing: true, - }), - ), - ) - ).flat(); - - const violations = []; - for (const filePath of files) { - const content = await fs.readFile(filePath, "utf8"); - const relPath = path.relative(repoRoot, filePath).replaceAll(path.sep, "/"); - for (const line of findRawFetchCallLines(content, filePath)) { - const callsite = `${relPath}:${line}`; - if (allowedRawFetchCallsites.has(callsite)) { - continue; - } - violations.push(callsite); - } - } - - if (violations.length === 0) { - return; - } - - console.error("Found raw fetch() usage in channel/plugin runtime sources outside allowlist:"); - for (const violation of violations.toSorted()) { - console.error(`- ${violation}`); - } - console.error( - "Use fetchWithSsrFGuard() or existing channel/plugin SDK wrappers for network calls.", - ); - process.exit(1); + await runCallsiteGuard({ + importMetaUrl: import.meta.url, + sourceRoots, + extraTestSuffixes: [".browser.test.ts", ".node.test.ts"], + findCallLines: findRawFetchCallLines, + allowCallsite: (callsite) => allowedRawFetchCallsites.has(callsite), + header: "Found raw fetch() usage in channel/plugin runtime sources outside allowlist:", + footer: "Use fetchWithSsrFGuard() or existing channel/plugin SDK wrappers for network calls.", + }); } runAsScript(import.meta.url, main); diff --git a/scripts/check-pairing-account-scope.mjs b/scripts/check-pairing-account-scope.mjs index 984e3846fc6..83a10750625 100644 --- a/scripts/check-pairing-account-scope.mjs +++ b/scripts/check-pairing-account-scope.mjs @@ -1,17 +1,15 @@ #!/usr/bin/env node -import path from "node:path"; import ts from "typescript"; +import { createPairingGuardContext } from "./lib/pairing-guard-context.mjs"; import { collectFileViolations, getPropertyNameText, - resolveRepoRoot, runAsScript, toLine, } from "./lib/ts-guard-utils.mjs"; -const repoRoot = resolveRepoRoot(import.meta.url); -const sourceRoots = [path.join(repoRoot, "src"), path.join(repoRoot, "extensions")]; +const { repoRoot, sourceRoots } = createPairingGuardContext(import.meta.url); function isUndefinedLikeExpression(node) { if (ts.isIdentifier(node) && node.text === "undefined") { diff --git a/scripts/lib/callsite-guard.mjs b/scripts/lib/callsite-guard.mjs new file mode 100644 index 00000000000..94715e9cb9b --- /dev/null +++ b/scripts/lib/callsite-guard.mjs @@ -0,0 +1,45 @@ +import { promises as fs } from "node:fs"; +import path from "node:path"; +import { + collectTypeScriptFilesFromRoots, + resolveRepoRoot, + resolveSourceRoots, +} from "./ts-guard-utils.mjs"; + +export async function runCallsiteGuard(params) { + const repoRoot = resolveRepoRoot(params.importMetaUrl); + const sourceRoots = resolveSourceRoots(repoRoot, params.sourceRoots); + const files = await collectTypeScriptFilesFromRoots(sourceRoots, { + extraTestSuffixes: params.extraTestSuffixes, + }); + const violations = []; + + for (const filePath of files) { + const relPath = path.relative(repoRoot, filePath).replaceAll(path.sep, "/"); + if (params.skipRelativePath?.(relPath)) { + continue; + } + const content = await fs.readFile(filePath, "utf8"); + for (const line of params.findCallLines(content, filePath)) { + const callsite = `${relPath}:${line}`; + if (params.allowCallsite?.(callsite)) { + continue; + } + violations.push(callsite); + } + } + + if (violations.length === 0) { + return; + } + + console.error(params.header); + const output = params.sortViolations === false ? violations : violations.toSorted(); + for (const violation of output) { + console.error(`- ${violation}`); + } + if (params.footer) { + console.error(params.footer); + } + process.exit(1); +} diff --git a/scripts/lib/pairing-guard-context.mjs b/scripts/lib/pairing-guard-context.mjs new file mode 100644 index 00000000000..e34df00529c --- /dev/null +++ b/scripts/lib/pairing-guard-context.mjs @@ -0,0 +1,13 @@ +import path from "node:path"; +import { resolveRepoRoot, resolveSourceRoots } from "./ts-guard-utils.mjs"; + +export function createPairingGuardContext(importMetaUrl) { + const repoRoot = resolveRepoRoot(importMetaUrl); + const sourceRoots = resolveSourceRoots(repoRoot, ["src", "extensions"]); + return { + repoRoot, + sourceRoots, + resolveFromRepo: (relativePath) => + path.join(repoRoot, ...relativePath.split("/").filter(Boolean)), + }; +} diff --git a/scripts/lib/ts-guard-utils.mjs b/scripts/lib/ts-guard-utils.mjs index bdf69246c56..0bbb81cc45c 100644 --- a/scripts/lib/ts-guard-utils.mjs +++ b/scripts/lib/ts-guard-utils.mjs @@ -9,6 +9,10 @@ export function resolveRepoRoot(importMetaUrl) { return path.resolve(path.dirname(fileURLToPath(importMetaUrl)), "..", ".."); } +export function resolveSourceRoots(repoRoot, relativeRoots) { + return relativeRoots.map((root) => path.join(repoRoot, ...root.split("/").filter(Boolean))); +} + export function isTestLikeTypeScriptFile(filePath, options = {}) { const extraTestSuffixes = options.extraTestSuffixes ?? []; return [...baseTestSuffixes, ...extraTestSuffixes].some((suffix) => filePath.endsWith(suffix)); @@ -68,18 +72,24 @@ export async function collectTypeScriptFiles(targetPath, options = {}) { return out; } -export async function collectFileViolations(params) { - const files = ( +export async function collectTypeScriptFilesFromRoots(sourceRoots, options = {}) { + return ( await Promise.all( - params.sourceRoots.map( + sourceRoots.map( async (root) => await collectTypeScriptFiles(root, { ignoreMissing: true, - extraTestSuffixes: params.extraTestSuffixes, + ...options, }), ), ) ).flat(); +} + +export async function collectFileViolations(params) { + const files = await collectTypeScriptFilesFromRoots(params.sourceRoots, { + extraTestSuffixes: params.extraTestSuffixes, + }); const violations = []; for (const filePath of files) {