mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 19:35:04 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The server heartbeat service dispatches scheduled retries and queued runs. > - The service kept policy decisions and database writes in one large file. > - This layout made policy branches harder to test and transaction boundaries harder to inspect. > - This pull request moves the policy rules and database transactions into a run-dispatch module. > - The benefit is a smaller service, pure policy tests, and clear transaction ownership. ## Linked Issues or Issue Description **What existing behavior does this improve?** The server heartbeat service promotes scheduled retries and cancels stale queued runs. **Subsystem affected** server/ — REST API and orchestration services. **Current behavior** The heartbeat service contains the policy rules and the database writes for these dispatch paths. **Proposed behavior** A run-dispatch module owns pure policy functions and semantic database transactions. The public service contracts stay unchanged. **Reason and benefit** The new layout separates branch rules from database effects. It makes each policy branch easier to test and keeps each operation’s row writes in one transaction. **Breaking changes** None. The public service contracts stay unchanged. ## What Changed - Move scheduled-retry promotion and queued-run staleness rules into pure functions. - Add table-driven unit tests for each policy branch. - Move promotion and cancellation writes into semantic transactions. - Keep row locking, company isolation, and post-commit effects unchanged. ## Verification - `node scripts/check-module-boundaries.mjs` passes. - `tsc --noEmit` from `server/` reports no errors. - The focused server test command passes 228 tests in six files. - Full pull request CI passes. - Greptile reports 5/5, and all review threads are resolved. ## Risks The main risk concerns changed transaction boundaries in scheduled-retry promotion and queued-run cancellation. The focused tests retain coverage for locking, transactionality, company isolation, and transport contracts. The public service contracts do not change. ## Model Used Codex, GPT-5, with code execution and tool use. The context window is not provided by the runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes: #` / `Refs: #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I have addressed all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
169 lines
6.3 KiB
JavaScript
169 lines
6.3 KiB
JavaScript
#!/usr/bin/env node
|
|
|
|
import { readdirSync, readFileSync } from "node:fs";
|
|
import { dirname, relative, resolve, sep } from "node:path";
|
|
import { fileURLToPath } from "node:url";
|
|
|
|
const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), "..");
|
|
const defaultServerSrc = resolve(repoRoot, "server/src");
|
|
const defaultModulesRoot = resolve(defaultServerSrc, "modules");
|
|
const layerNames = new Set(["domain", "application", "adapters"]);
|
|
const databasePackages = ["@paperclipai/db", "drizzle-orm", "embedded-postgres", "postgres"];
|
|
|
|
function normalizedRelative(from, to) {
|
|
return relative(from, to).split(sep).join("/");
|
|
}
|
|
|
|
function isInside(root, candidate) {
|
|
const rel = relative(root, candidate);
|
|
return rel === "" || (!rel.startsWith(`..${sep}`) && rel !== "..");
|
|
}
|
|
|
|
function isPackageOrSubpath(specifier, packageName) {
|
|
return specifier === packageName || specifier.startsWith(`${packageName}/`);
|
|
}
|
|
|
|
function isDatabasePackage(specifier) {
|
|
return databasePackages.some((packageName) => isPackageOrSubpath(specifier, packageName));
|
|
}
|
|
|
|
export function extractImportSpecifiers(sourceText) {
|
|
const specifiers = new Set();
|
|
const patterns = [
|
|
/\bimport\s+(?:type\s+)?(?:[^"'`;]*?\s+from\s+)?["']([^"']+)["']/g,
|
|
/\bexport\s+(?:type\s+)?(?:[^"'`;]*?\s+from\s+)?["']([^"']+)["']/g,
|
|
/\bimport\s*\(\s*["']([^"']+)["']\s*\)/g,
|
|
/\brequire\s*\(\s*["']([^"']+)["']\s*\)/g,
|
|
];
|
|
|
|
for (const pattern of patterns) {
|
|
for (const match of sourceText.matchAll(pattern)) specifiers.add(match[1]);
|
|
}
|
|
return [...specifiers];
|
|
}
|
|
|
|
function listProductionSourceFiles(root) {
|
|
const files = [];
|
|
const walk = (directory) => {
|
|
for (const entry of readdirSync(directory, { withFileTypes: true })) {
|
|
if (entry.name === "node_modules" || entry.name === "dist") continue;
|
|
const entryPath = resolve(directory, entry.name);
|
|
if (entry.isDirectory()) walk(entryPath);
|
|
else if (
|
|
entry.isFile() &&
|
|
/\.(?:ts|tsx)$/.test(entry.name) &&
|
|
!/\.(?:test|spec)\.(?:ts|tsx)$/.test(entry.name) &&
|
|
!entry.name.endsWith(".d.ts")
|
|
) {
|
|
files.push(entryPath);
|
|
}
|
|
}
|
|
};
|
|
walk(root);
|
|
return files.sort();
|
|
}
|
|
|
|
function moduleLocation(modulesRoot, filePath) {
|
|
if (!isInside(modulesRoot, filePath)) return null;
|
|
const [moduleName, layer] = normalizedRelative(modulesRoot, filePath).split("/");
|
|
if (!moduleName) return null;
|
|
return { moduleName, layer: layerNames.has(layer) ? layer : null };
|
|
}
|
|
|
|
function resolvedTarget(sourceFile, specifier) {
|
|
return specifier.startsWith(".") ? resolve(dirname(sourceFile), specifier) : null;
|
|
}
|
|
|
|
function targetServerSegments(serverSrc, target) {
|
|
if (!target || !isInside(serverSrc, target)) return [];
|
|
return normalizedRelative(serverSrc, target).split("/");
|
|
}
|
|
|
|
function addViolation(violations, file, layer, specifier, reason) {
|
|
violations.push({ file, layer, specifier, reason });
|
|
}
|
|
|
|
export function scanModuleBoundaries({
|
|
serverSrc = defaultServerSrc,
|
|
modulesRoot = defaultModulesRoot,
|
|
} = {}) {
|
|
const violations = [];
|
|
|
|
for (const sourceFile of listProductionSourceFiles(serverSrc)) {
|
|
const sourceLocation = moduleLocation(modulesRoot, sourceFile);
|
|
const sourceLabel = normalizedRelative(repoRoot, sourceFile);
|
|
const sourceText = readFileSync(sourceFile, "utf8");
|
|
|
|
for (const specifier of extractImportSpecifiers(sourceText)) {
|
|
const target = resolvedTarget(sourceFile, specifier);
|
|
const targetSegments = targetServerSegments(serverSrc, target);
|
|
const targetLocation = target ? moduleLocation(modulesRoot, target) : null;
|
|
|
|
if (sourceLocation?.layer === "domain") {
|
|
if (isDatabasePackage(specifier)) {
|
|
addViolation(violations, sourceLabel, "domain", specifier, "domain cannot import database packages");
|
|
} else if (specifier.startsWith("node:")) {
|
|
addViolation(violations, sourceLabel, "domain", specifier, "domain cannot import Node.js runtime modules");
|
|
} else if (
|
|
targetSegments.includes("services") ||
|
|
targetSegments.includes("routes") ||
|
|
targetSegments.includes("adapters")
|
|
) {
|
|
addViolation(
|
|
violations,
|
|
sourceLabel,
|
|
"domain",
|
|
specifier,
|
|
"domain cannot import server services, routes, or adapters",
|
|
);
|
|
} else if (targetLocation?.layer === "application" || targetLocation?.layer === "adapters") {
|
|
addViolation(violations, sourceLabel, "domain", specifier, "domain cannot depend on outer module layers");
|
|
}
|
|
}
|
|
|
|
if (sourceLocation?.layer === "application") {
|
|
if (isDatabasePackage(specifier)) {
|
|
addViolation(violations, sourceLabel, "application", specifier, "application cannot import database packages");
|
|
} else if (targetLocation?.layer === "adapters" || targetSegments.includes("adapters")) {
|
|
addViolation(violations, sourceLabel, "application", specifier, "application cannot import concrete adapters");
|
|
} else if (targetSegments.join("/") === "errors.js" || targetSegments.join("/") === "errors.ts") {
|
|
addViolation(violations, sourceLabel, "application", specifier, "application cannot import HTTP error helpers");
|
|
}
|
|
}
|
|
|
|
if (targetLocation && sourceLocation?.moduleName !== targetLocation.moduleName) {
|
|
const targetRelative = normalizedRelative(resolve(modulesRoot, targetLocation.moduleName), target);
|
|
if (targetRelative !== "index.js" && targetRelative !== "index.ts") {
|
|
addViolation(
|
|
violations,
|
|
sourceLabel,
|
|
sourceLocation?.layer ?? null,
|
|
specifier,
|
|
`imports inside module ${targetLocation.moduleName} instead of its index`,
|
|
);
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
return violations;
|
|
}
|
|
|
|
export function formatViolation(violation) {
|
|
const layer = violation.layer ? ` (${violation.layer})` : "";
|
|
return `${violation.file}${layer}: ${violation.reason}: ${JSON.stringify(violation.specifier)}`;
|
|
}
|
|
|
|
function main() {
|
|
const violations = scanModuleBoundaries();
|
|
if (violations.length > 0) {
|
|
console.error("Feature module boundary check failed:");
|
|
for (const violation of violations) console.error(`- ${formatViolation(violation)}`);
|
|
process.exitCode = 1;
|
|
return;
|
|
}
|
|
console.log("Feature module boundary check passed.");
|
|
}
|
|
|
|
if (resolve(process.argv[1] ?? "") === fileURLToPath(import.meta.url)) main();
|