mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-02 02:07:25 +08:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The heartbeat service admits wake requests while an issue has an active execution run. > - That admission branch mixes wake policy, database reads, and database writes in one service. > - This structure makes the wake-queue boundary hard to test and extend. > - This pull request moves the admission policy and its database adapter into the wake-queue module. > - The result keeps heartbeat orchestration small and makes the admission behavior testable in isolation. ## Linked Issues or Issue Description **What existing behavior does this improve?** The heartbeat service now delegates deferred wake admission to the wake-queue module. The module keeps the existing merge, defer, and ordinary-wake outcomes. **Subsystem affected** server/ — REST API and orchestration services. **Current behavior** The heartbeat service contains a 146-line branch that reads wake state, chooses an outcome, and writes the result. **Proposed behavior** The wake-queue module owns the pure admission decision and the adapter reads and writes. The heartbeat service calls one module method. **Reason and benefit** This boundary reduces service coupling and lets module tests cover the admission policy. The change keeps the existing reason strings and outcomes. **Breaking changes** None. The change preserves the current behavior and public API. **Additional context** This pull request follows [PR #13132](https://github.com/paperclipai/paperclip/pull/13132), which merged the first slice of this refactor. I searched GitHub for duplicate and related pull requests before opening this pull request. ## What Changed - Move deferred wake admission policy into `server/src/modules/wake-queue`. - Add module ports and a PostgreSQL adapter for the admission reads and writes. - Keep the existing wake outcomes and stored reason strings. - Extend the module boundary check to reject service imports from the application layer. - Add unit and adapter tests for the moved behavior. ## Verification - `node --test scripts/check-module-boundaries.test.mjs` passes. - The `server/src/modules/wake-queue` suite passes 58 tests. - The eight pinned heartbeat and queued-comment tests remain unchanged and require CI verification. - Every continuous-integration check must reach a terminal green state before merge. ## Risks The refactor changes the location of wake admission logic. A missed adapter condition could change deferred wake behavior. The tests cover the policy outcomes and the adapter writes. The residual tenant-scope risk remains documented in the review record. ## Model Used OpenAI Codex, GPT-5, tool use and code execution. The implementation author ran the tests and prepared the commit set. ## 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 will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
171 lines
6.5 KiB
JavaScript
171 lines
6.5 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.includes("services") || targetSegments.includes("routes")) {
|
|
addViolation(violations, sourceLabel, "application", specifier, "application cannot import server services or routes");
|
|
} 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();
|