mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-21 04:50:37 +09:00
Compare commits
1 commit
06a9428a36
...
4272b31136
| Author | SHA1 | Date | |
|---|---|---|---|
| 4272b31136 |
16 changed files with 116 additions and 20382 deletions
53
.github/scripts/check-pr-linked-issue.mjs
vendored
53
.github/scripts/check-pr-linked-issue.mjs
vendored
|
|
@ -1,53 +0,0 @@
|
|||
#!/usr/bin/env node
|
||||
/**
|
||||
* check-pr-linked-issue.mjs
|
||||
* Checks that a PR body references a GitHub issue. Respects conventional commit
|
||||
* prefixes — skips check for docs/chore/build/ci/style/test prefixed PRs.
|
||||
* Export: checkLinkedIssue(prBody: string, prTitle: string) → { passed, failures }
|
||||
*/
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
const ISSUE_PATTERNS = [
|
||||
/(?:fixes|closes|resolves)\s+#\d+/i,
|
||||
/(?:^|[\s(])https:\/\/github\.com\/paperclipai\/paperclip\/issues\/\d+(?=$|[\s),:;!?]|[.](?![\w-]))/i,
|
||||
/(?<!\w)#\d+/,
|
||||
];
|
||||
|
||||
// Prefixes where a linked issue is NOT required
|
||||
const SKIP_ISSUE_PREFIXES = ['docs', 'chore', 'build', 'ci', 'style', 'test', 'revert'];
|
||||
|
||||
function parsePrefix(title) {
|
||||
if (!title) return null;
|
||||
const match = title.match(/^([a-z]+)(?:\([^)]*\))?:/);
|
||||
return match ? match[1].toLowerCase() : null;
|
||||
}
|
||||
|
||||
export function checkLinkedIssue(body, prTitle = '') {
|
||||
const prefix = parsePrefix(prTitle);
|
||||
|
||||
if (prefix && SKIP_ISSUE_PREFIXES.includes(prefix)) {
|
||||
return { passed: true, failures: [] };
|
||||
}
|
||||
|
||||
if (!body || !body.trim()) {
|
||||
return { passed: false, failures: ['PR body is empty — please fill out the PR template'] };
|
||||
}
|
||||
|
||||
const found = ISSUE_PATTERNS.some(p => p.test(body));
|
||||
return {
|
||||
passed: found,
|
||||
failures: found ? [] : [
|
||||
'No linked issue found — please add `Fixes #NNN` to your PR description. ' +
|
||||
'If no issue exists yet, please file one first: ' +
|
||||
'https://github.com/paperclipai/paperclip/issues/new',
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
if (process.argv[1] === fileURLToPath(import.meta.url)) {
|
||||
const body = process.env.PR_BODY ?? '';
|
||||
const title = process.env.PR_TITLE ?? '';
|
||||
const result = checkLinkedIssue(body, title);
|
||||
console.log(JSON.stringify(result));
|
||||
process.exit(result.passed ? 0 : 1);
|
||||
}
|
||||
18
.github/scripts/check-pr-security.mjs
vendored
18
.github/scripts/check-pr-security.mjs
vendored
|
|
@ -225,14 +225,10 @@ export async function syncDraftAdvisory(fetchImpl, token, repo, prNumber, prTitl
|
|||
throw new Error(`Existing advisory for PR #${prNumber} is missing both ghsa_id and id.`);
|
||||
}
|
||||
|
||||
// PATCH rejects `vulnerabilities: []` with 422 ("Advisory must have at least one vulnerability").
|
||||
// The field is only valid on POST when creating the draft; updates must omit it.
|
||||
const { vulnerabilities, ...patchPayload } = payload;
|
||||
|
||||
return fetchImpl(`/repos/${repo}/security-advisories/${advisoryId}`, token, {
|
||||
method: 'PATCH',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify(patchPayload),
|
||||
body: JSON.stringify(payload),
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -270,16 +266,10 @@ export async function postSecurityCheckRun(fetchImpl, token, repo, headSha, hasF
|
|||
body: JSON.stringify(hasFlags ? {
|
||||
name: 'security-review',
|
||||
head_sha: headSha,
|
||||
// `completed/neutral` instead of `in_progress` so the check doesn't put
|
||||
// the PR in `mergeStateStatus: BLOCKED`. The draft advisory is the
|
||||
// durable signal for maintainers; there is no completion path that
|
||||
// could ever flip an `in_progress` check-run back to completed on the
|
||||
// same head SHA, so it would hang forever.
|
||||
status: 'completed',
|
||||
conclusion: 'neutral',
|
||||
status: 'in_progress',
|
||||
output: {
|
||||
title: 'Security Review Recommended',
|
||||
summary: 'Draft advisory filed for maintainer review. Not a merge block — review the advisory at your leisure.',
|
||||
title: 'Security Review Pending',
|
||||
summary: 'This PR has been flagged for manual security review by a maintainer. No action needed from you.',
|
||||
},
|
||||
} : {
|
||||
name: 'security-review',
|
||||
|
|
|
|||
5
.github/scripts/run-quality-gates.mjs
vendored
5
.github/scripts/run-quality-gates.mjs
vendored
|
|
@ -11,7 +11,6 @@ import { fileURLToPath } from 'node:url';
|
|||
import { ghFetch } from './get-bot-token.mjs';
|
||||
import { fetchAllPullRequestFiles } from './fetch-pr-files.mjs';
|
||||
import { checkTemplate } from './check-pr-template.mjs';
|
||||
import { checkLinkedIssue } from './check-pr-linked-issue.mjs';
|
||||
import { checkTestCoverage } from './check-pr-test-coverage.mjs';
|
||||
import { checkLockfile } from './check-pr-lockfile.mjs';
|
||||
import { checkDependencies } from './check-pr-dependencies.mjs';
|
||||
|
|
@ -110,10 +109,9 @@ async function main() {
|
|||
|
||||
// Run all quality gates (pure functions run sync, deps check is async)
|
||||
const prTitle = pr.title ?? '';
|
||||
const [templateResult, issueResult, testResult, lockfileResult, depsResult] =
|
||||
const [templateResult, testResult, lockfileResult, depsResult] =
|
||||
await Promise.all([
|
||||
Promise.resolve(checkTemplate(prBody)),
|
||||
Promise.resolve(checkLinkedIssue(prBody, prTitle)),
|
||||
Promise.resolve(checkTestCoverage(files, prTitle)),
|
||||
Promise.resolve(checkLockfile(files, author, branch)),
|
||||
checkDependencies(files, GH_TOKEN, GH_REPO, prNumber, pr.base?.ref),
|
||||
|
|
@ -121,7 +119,6 @@ async function main() {
|
|||
|
||||
const allFailures = [
|
||||
...templateResult.failures,
|
||||
...issueResult.failures,
|
||||
...testResult.failures,
|
||||
...lockfileResult.failures,
|
||||
];
|
||||
|
|
|
|||
111
.github/scripts/tests/check-pr-linked-issue.test.mjs
vendored
111
.github/scripts/tests/check-pr-linked-issue.test.mjs
vendored
|
|
@ -1,111 +0,0 @@
|
|||
import { test } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { checkLinkedIssue } from '../check-pr-linked-issue.mjs';
|
||||
|
||||
// Existing tests with title parameter added (defaults to no prefix, so still required)
|
||||
|
||||
test('passes with bare #NNN reference', () => {
|
||||
assert.equal(checkLinkedIssue('This fixes the bug in #123', 'fix: something').passed, true);
|
||||
});
|
||||
|
||||
test('passes with "Fixes #NNN"', () => {
|
||||
assert.equal(checkLinkedIssue('Fixes #456\n\nSome description', 'fix: something').passed, true);
|
||||
});
|
||||
|
||||
test('passes with "Closes #NNN" (case-insensitive)', () => {
|
||||
assert.equal(checkLinkedIssue('closes #789', 'fix: something').passed, true);
|
||||
});
|
||||
|
||||
test('passes with "Resolves #NNN"', () => {
|
||||
assert.equal(checkLinkedIssue('Resolves #101', 'fix: something').passed, true);
|
||||
});
|
||||
|
||||
test('passes with full github.com URL', () => {
|
||||
assert.equal(
|
||||
checkLinkedIssue('See https://github.com/paperclipai/paperclip/issues/202', 'fix: bug').passed,
|
||||
true
|
||||
);
|
||||
});
|
||||
|
||||
test('passes with a full github.com URL followed by punctuation', () => {
|
||||
assert.equal(
|
||||
checkLinkedIssue('See (https://github.com/paperclipai/paperclip/issues/202).', 'fix: bug').passed,
|
||||
true
|
||||
);
|
||||
});
|
||||
|
||||
test('fails with empty body when no skip prefix', () => {
|
||||
const result = checkLinkedIssue('', 'fix: bug');
|
||||
assert.equal(result.passed, false);
|
||||
assert.ok(result.failures.length > 0);
|
||||
});
|
||||
|
||||
test('fails with no issue reference when no skip prefix', () => {
|
||||
const result = checkLinkedIssue('Added a cool feature, no issue linked', 'feat: something');
|
||||
assert.equal(result.passed, false);
|
||||
assert.ok(result.failures[0].includes('Fixes #NNN'));
|
||||
});
|
||||
|
||||
test('fails with cross-repo issue reference', () => {
|
||||
const result = checkLinkedIssue('See https://github.com/other/repo/issues/123', 'fix: bug');
|
||||
assert.equal(result.passed, false);
|
||||
});
|
||||
|
||||
test('fails when the Paperclip issue URL is embedded inside another host', () => {
|
||||
const result = checkLinkedIssue(
|
||||
'See https://evil.example/https://github.com/paperclipai/paperclip/issues/123',
|
||||
'fix: bug'
|
||||
);
|
||||
assert.equal(result.passed, false);
|
||||
});
|
||||
|
||||
test('fails when the Paperclip issue URL continues into another host', () => {
|
||||
const result = checkLinkedIssue(
|
||||
'See https://github.com/paperclipai/paperclip/issues/123.evil.example',
|
||||
'fix: bug'
|
||||
);
|
||||
assert.equal(result.passed, false);
|
||||
});
|
||||
|
||||
test('fails when #NNN is part of a word (no space before)', () => {
|
||||
const result = checkLinkedIssue('This is version#123 not an issue link', 'fix: bug');
|
||||
assert.equal(result.passed, false);
|
||||
});
|
||||
|
||||
// New tests for prefix-aware skip behavior
|
||||
|
||||
test('skips check for docs: prefix', () => {
|
||||
assert.equal(checkLinkedIssue('', 'docs: update README').passed, true);
|
||||
});
|
||||
|
||||
test('skips check for chore: prefix', () => {
|
||||
assert.equal(checkLinkedIssue('', 'chore: bump deps').passed, true);
|
||||
});
|
||||
|
||||
test('skips check for build: prefix', () => {
|
||||
assert.equal(checkLinkedIssue('', 'build: update Dockerfile').passed, true);
|
||||
});
|
||||
|
||||
test('skips check for ci: prefix', () => {
|
||||
assert.equal(checkLinkedIssue('', 'ci: add workflow').passed, true);
|
||||
});
|
||||
|
||||
test('skips check for test: prefix', () => {
|
||||
assert.equal(checkLinkedIssue('', 'test: add coverage').passed, true);
|
||||
});
|
||||
|
||||
test('skips check with scoped prefix like docs(api):', () => {
|
||||
assert.equal(checkLinkedIssue('', 'docs(api): document endpoint').passed, true);
|
||||
});
|
||||
|
||||
test('requires issue for feat: prefix', () => {
|
||||
assert.equal(checkLinkedIssue('Some description without issue', 'feat: new thing').passed, false);
|
||||
});
|
||||
|
||||
test('requires issue for refactor: prefix', () => {
|
||||
assert.equal(checkLinkedIssue('Some refactor', 'refactor: rewrite thing').passed, false);
|
||||
});
|
||||
|
||||
test('requires issue when no prefix (encourages prefix usage)', () => {
|
||||
assert.equal(checkLinkedIssue('No prefix here', 'Add some feature').passed, false);
|
||||
});
|
||||
12
.github/scripts/tests/check-pr-security.test.mjs
vendored
12
.github/scripts/tests/check-pr-security.test.mjs
vendored
|
|
@ -161,10 +161,7 @@ test('syncDraftAdvisory: patches an existing advisory with the latest flags', as
|
|||
assert.equal(calls.length, 2);
|
||||
assert.equal(calls[1].path, '/repos/paperclipai/paperclip/security-advisories/GHSA-test-1234');
|
||||
assert.equal(calls[1].options.method, 'PATCH');
|
||||
const patchBody = JSON.parse(calls[1].options.body);
|
||||
const { vulnerabilities, ...expectedPatch } = buildAdvisoryPayload(6469, 'My PR', flags);
|
||||
assert.deepEqual(patchBody, expectedPatch);
|
||||
assert.ok(!('vulnerabilities' in patchBody), 'PATCH must omit vulnerabilities (GitHub rejects empty array with 422)');
|
||||
assert.deepEqual(JSON.parse(calls[1].options.body), buildAdvisoryPayload(6469, 'My PR', flags));
|
||||
});
|
||||
|
||||
test('syncDraftAdvisory: creates a new advisory when none exists', async () => {
|
||||
|
|
@ -199,11 +196,10 @@ test('postSecurityCheckRun: uses the injected fetch implementation', async () =>
|
|||
assert.deepEqual(JSON.parse(calls[0].options.body), {
|
||||
name: 'security-review',
|
||||
head_sha: 'deadbeef',
|
||||
status: 'completed',
|
||||
conclusion: 'neutral',
|
||||
status: 'in_progress',
|
||||
output: {
|
||||
title: 'Security Review Recommended',
|
||||
summary: 'Draft advisory filed for maintainer review. Not a merge block — review the advisory at your leisure.',
|
||||
title: 'Security Review Pending',
|
||||
summary: 'This PR has been flagged for manual security review by a maintainer. No action needed from you.',
|
||||
},
|
||||
});
|
||||
});
|
||||
|
|
|
|||
5
.github/workflows/commitperclip-review.yml
vendored
5
.github/workflows/commitperclip-review.yml
vendored
|
|
@ -45,7 +45,6 @@ jobs:
|
|||
|
||||
- name: Run quality gates
|
||||
id: quality
|
||||
if: github.event.pull_request.user.login != 'dependabot[bot]'
|
||||
run: node .github/scripts/run-quality-gates.mjs
|
||||
continue-on-error: true
|
||||
env:
|
||||
|
|
@ -64,9 +63,7 @@ jobs:
|
|||
PR_AUTHOR: ${{ github.event.pull_request.user.login }}
|
||||
|
||||
- name: Fail if quality gates failed
|
||||
if: >-
|
||||
github.event.pull_request.user.login != 'dependabot[bot]' &&
|
||||
steps.quality.outcome == 'failure'
|
||||
if: steps.quality.outcome == 'failure'
|
||||
run: |
|
||||
echo "One or more quality gates failed. See commitperclip comment on the PR for details."
|
||||
exit 1
|
||||
|
|
|
|||
4
.github/workflows/pr.yml
vendored
4
.github/workflows/pr.yml
vendored
|
|
@ -21,9 +21,7 @@ jobs:
|
|||
fetch-depth: 0
|
||||
|
||||
- name: Block manual lockfile edits
|
||||
if: >-
|
||||
github.head_ref != 'chore/refresh-lockfile' &&
|
||||
github.event.pull_request.user.login != 'dependabot[bot]'
|
||||
if: github.head_ref != 'chore/refresh-lockfile'
|
||||
run: |
|
||||
# Diff the PR branch against its merge base so recent base-branch commits
|
||||
# do not masquerade as changes made by the PR itself.
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load diff
|
|
@ -419,10 +419,8 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost
|
|||
|
||||
config: {
|
||||
async get(params) {
|
||||
if (params && "companyId" in params) {
|
||||
return callHost("config.get", { companyId: params.companyId ?? null });
|
||||
}
|
||||
const companyId = runtimeCompanyContext.getStore()?.companyId ?? null;
|
||||
const companyId =
|
||||
params?.companyId ?? runtimeCompanyContext.getStore()?.companyId ?? null;
|
||||
return callHost("config.get", companyId ? { companyId } : {});
|
||||
},
|
||||
},
|
||||
|
|
@ -574,9 +572,7 @@ export function startWorkerRpcHost(options: WorkerRpcHostOptions): WorkerRpcHost
|
|||
|
||||
secrets: {
|
||||
async resolve(secretRef: string, companyId?: string | null): Promise<string> {
|
||||
const scopedCompanyId = arguments.length >= 2
|
||||
? companyId ?? null
|
||||
: runtimeCompanyContext.getStore()?.companyId ?? null;
|
||||
const scopedCompanyId = companyId ?? runtimeCompanyContext.getStore()?.companyId ?? null;
|
||||
return callHost("secrets.resolve", { secretRef, companyId: scopedCompanyId });
|
||||
},
|
||||
},
|
||||
|
|
|
|||
|
|
@ -355,163 +355,65 @@ describe("startWorkerRpcHost runtime company context", () => {
|
|||
|
||||
const host = startWorkerRpcHost({ plugin, stdin, stdout });
|
||||
|
||||
try {
|
||||
writeMessage(stdin, {
|
||||
jsonrpc: "2.0",
|
||||
id: 1,
|
||||
method: "initialize",
|
||||
params: {
|
||||
manifest: { id: "test-plugin", name: "test-plugin", version: "1.0.0" },
|
||||
config: {},
|
||||
instanceInfo: { instanceId: "inst-1", hostVersion: "0.0.0-test" },
|
||||
apiVersion: 1,
|
||||
},
|
||||
});
|
||||
await expect(nextMessage()).resolves.toMatchObject({ id: 1, result: { ok: true } });
|
||||
writeMessage(stdin, {
|
||||
jsonrpc: "2.0",
|
||||
id: 1,
|
||||
method: "initialize",
|
||||
params: {
|
||||
manifest: { id: "test-plugin", name: "test-plugin", version: "1.0.0" },
|
||||
config: {},
|
||||
instanceInfo: { instanceId: "inst-1", hostVersion: "0.0.0-test" },
|
||||
apiVersion: 1,
|
||||
},
|
||||
});
|
||||
await expect(nextMessage()).resolves.toMatchObject({ id: 1, result: { ok: true } });
|
||||
|
||||
writeMessage(stdin, {
|
||||
jsonrpc: "2.0",
|
||||
id: 2,
|
||||
method: "executeTool",
|
||||
params: {
|
||||
toolName: "check-context",
|
||||
parameters: {},
|
||||
runContext: {
|
||||
agentId: "agent-1",
|
||||
runId: "run-1",
|
||||
companyId: "company-1",
|
||||
projectId: "project-1",
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const configRequest = await nextMessage();
|
||||
expect(configRequest).toMatchObject({
|
||||
method: "config.get",
|
||||
params: { companyId: "company-1" },
|
||||
});
|
||||
writeMessage(stdin, {
|
||||
jsonrpc: "2.0",
|
||||
id: configRequest.id,
|
||||
result: { mode: "company-config" },
|
||||
});
|
||||
|
||||
const secretRequest = await nextMessage();
|
||||
expect(secretRequest).toMatchObject({
|
||||
method: "secrets.resolve",
|
||||
params: {
|
||||
secretRef: "77777777-7777-4777-8777-777777777777",
|
||||
writeMessage(stdin, {
|
||||
jsonrpc: "2.0",
|
||||
id: 2,
|
||||
method: "executeTool",
|
||||
params: {
|
||||
toolName: "check-context",
|
||||
parameters: {},
|
||||
runContext: {
|
||||
agentId: "agent-1",
|
||||
runId: "run-1",
|
||||
companyId: "company-1",
|
||||
projectId: "project-1",
|
||||
},
|
||||
});
|
||||
writeMessage(stdin, {
|
||||
jsonrpc: "2.0",
|
||||
id: secretRequest.id,
|
||||
result: "company-secret",
|
||||
});
|
||||
|
||||
await expect(nextMessage()).resolves.toMatchObject({
|
||||
id: 2,
|
||||
result: { content: "company-config:company-secret" },
|
||||
});
|
||||
} finally {
|
||||
host.stop();
|
||||
stdin.destroy();
|
||||
stdout.destroy();
|
||||
}
|
||||
});
|
||||
|
||||
it("preserves explicit null company context in config and secret host calls", async () => {
|
||||
const stdin = new PassThrough();
|
||||
const stdout = new PassThrough();
|
||||
const nextMessage = collectJsonLines(stdout);
|
||||
|
||||
const plugin = definePlugin({
|
||||
async setup(ctx) {
|
||||
ctx.tools.register(
|
||||
"check-explicit-null",
|
||||
{
|
||||
displayName: "Check Explicit Null",
|
||||
description: "Checks explicit null runtime context propagation",
|
||||
parametersSchema: { type: "object", properties: {} },
|
||||
},
|
||||
async () => {
|
||||
const config = await ctx.config.get({ companyId: null });
|
||||
const token = await ctx.secrets.resolve(
|
||||
"77777777-7777-4777-8777-777777777777",
|
||||
null,
|
||||
);
|
||||
return { content: `${config.mode}:${token}` };
|
||||
},
|
||||
);
|
||||
},
|
||||
});
|
||||
|
||||
const host = startWorkerRpcHost({ plugin, stdin, stdout });
|
||||
const configRequest = await nextMessage();
|
||||
expect(configRequest).toMatchObject({
|
||||
method: "config.get",
|
||||
params: { companyId: "company-1" },
|
||||
});
|
||||
writeMessage(stdin, {
|
||||
jsonrpc: "2.0",
|
||||
id: configRequest.id,
|
||||
result: { mode: "company-config" },
|
||||
});
|
||||
|
||||
try {
|
||||
writeMessage(stdin, {
|
||||
jsonrpc: "2.0",
|
||||
id: 1,
|
||||
method: "initialize",
|
||||
params: {
|
||||
manifest: { id: "test-plugin", name: "test-plugin", version: "1.0.0" },
|
||||
config: {},
|
||||
instanceInfo: { instanceId: "inst-1", hostVersion: "0.0.0-test" },
|
||||
apiVersion: 1,
|
||||
},
|
||||
});
|
||||
await expect(nextMessage()).resolves.toMatchObject({ id: 1, result: { ok: true } });
|
||||
const secretRequest = await nextMessage();
|
||||
expect(secretRequest).toMatchObject({
|
||||
method: "secrets.resolve",
|
||||
params: {
|
||||
secretRef: "77777777-7777-4777-8777-777777777777",
|
||||
companyId: "company-1",
|
||||
},
|
||||
});
|
||||
writeMessage(stdin, {
|
||||
jsonrpc: "2.0",
|
||||
id: secretRequest.id,
|
||||
result: "company-secret",
|
||||
});
|
||||
|
||||
writeMessage(stdin, {
|
||||
jsonrpc: "2.0",
|
||||
id: 2,
|
||||
method: "executeTool",
|
||||
params: {
|
||||
toolName: "check-explicit-null",
|
||||
parameters: {},
|
||||
runContext: {
|
||||
agentId: "agent-1",
|
||||
runId: "run-1",
|
||||
companyId: "company-1",
|
||||
projectId: "project-1",
|
||||
},
|
||||
},
|
||||
});
|
||||
await expect(nextMessage()).resolves.toMatchObject({
|
||||
id: 2,
|
||||
result: { content: "company-config:company-secret" },
|
||||
});
|
||||
|
||||
const configRequest = await nextMessage();
|
||||
expect(configRequest).toMatchObject({
|
||||
method: "config.get",
|
||||
params: { companyId: null },
|
||||
});
|
||||
writeMessage(stdin, {
|
||||
jsonrpc: "2.0",
|
||||
id: configRequest.id,
|
||||
result: { mode: "global-config" },
|
||||
});
|
||||
|
||||
const secretRequest = await nextMessage();
|
||||
expect(secretRequest).toMatchObject({
|
||||
method: "secrets.resolve",
|
||||
params: {
|
||||
secretRef: "77777777-7777-4777-8777-777777777777",
|
||||
companyId: null,
|
||||
},
|
||||
});
|
||||
writeMessage(stdin, {
|
||||
jsonrpc: "2.0",
|
||||
id: secretRequest.id,
|
||||
result: "global-secret",
|
||||
});
|
||||
|
||||
await expect(nextMessage()).resolves.toMatchObject({
|
||||
id: 2,
|
||||
result: { content: "global-config:global-secret" },
|
||||
});
|
||||
} finally {
|
||||
host.stop();
|
||||
stdin.destroy();
|
||||
stdout.destroy();
|
||||
}
|
||||
host.stop();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,313 +0,0 @@
|
|||
/**
|
||||
* Regression + lifecycle coverage for plugin-loader → plugin-tool-dispatcher
|
||||
* → plugin-tool-registry → plugin-worker-manager UUID-keyed routing.
|
||||
*
|
||||
* Workers are keyed by DB UUID in PluginWorkerManager. If the dispatcher
|
||||
* registers tools without the UUID, `workerManager.isRunning(...)` checks
|
||||
* the pluginKey instead and always returns false, so every
|
||||
* /api/plugins/tools/execute returns 502 "worker for plugin X is not
|
||||
* running" even when the worker is alive. The dispatcher and registry
|
||||
* both require `pluginDbId` so this contract violation surfaces at the
|
||||
* call site instead of silently regressing.
|
||||
*
|
||||
* Covered paths:
|
||||
* 1. Activation (plugin-loader)
|
||||
* 2. Lifecycle (handlePluginEnabled / registerFromDb + initialize)
|
||||
* 3. Re-entry (disable → enable cycle, worker re-spawn,
|
||||
* idempotent re-register)
|
||||
* 4. Edge cases (missing UUID throws explicitly)
|
||||
*/
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { PaperclipPluginManifestV1 } from "@paperclipai/shared";
|
||||
import { EventEmitter } from "node:events";
|
||||
import { createPluginToolDispatcher } from "../services/plugin-tool-dispatcher.js";
|
||||
import type { PluginWorkerManager } from "../services/plugin-worker-manager.js";
|
||||
|
||||
const PLUGIN_KEY = "acme.demo";
|
||||
const PLUGIN_DB_ID = "00000000-0000-4000-8000-000000000001";
|
||||
|
||||
const MANIFEST: PaperclipPluginManifestV1 = {
|
||||
id: PLUGIN_KEY,
|
||||
apiVersion: 1,
|
||||
version: "1.0.0",
|
||||
displayName: "Demo plugin",
|
||||
description: "Regression fixture",
|
||||
author: "Paperclip",
|
||||
categories: ["automation"],
|
||||
capabilities: [],
|
||||
entrypoints: { worker: "dist/worker.js" },
|
||||
tools: [
|
||||
{
|
||||
name: "ping",
|
||||
displayName: "Ping",
|
||||
description: "Test tool",
|
||||
parametersSchema: { type: "object", properties: {} },
|
||||
},
|
||||
],
|
||||
} as unknown as PaperclipPluginManifestV1;
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Test doubles
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Stub worker manager whose `isRunning` only accepts the DB UUID. Any other
|
||||
* lookup key (notably the pluginKey) reports the worker as down — matches
|
||||
* the real `PluginWorkerManager` behavior which keys workers by UUID.
|
||||
*/
|
||||
function createUuidKeyedWorkerManager(opts: { liveUuid?: string } = {}): PluginWorkerManager {
|
||||
const liveUuid = opts.liveUuid ?? PLUGIN_DB_ID;
|
||||
const isRunning = vi.fn((id: string) => id === liveUuid);
|
||||
const call = vi.fn(async (id: string) => {
|
||||
if (!isRunning(id)) {
|
||||
throw new Error(`worker for plugin "${id}" is not running`);
|
||||
}
|
||||
return { ok: true } as unknown;
|
||||
});
|
||||
return {
|
||||
startWorker: vi.fn(),
|
||||
stopWorker: vi.fn(),
|
||||
getWorker: vi.fn(),
|
||||
isRunning,
|
||||
stopAll: vi.fn(),
|
||||
diagnostics: vi.fn(() => []),
|
||||
call,
|
||||
} as unknown as PluginWorkerManager;
|
||||
}
|
||||
|
||||
/**
|
||||
* In-memory lifecycle manager mirroring the real `PluginLifecycleManager`
|
||||
* event-emitter contract used by the dispatcher (plugin.enabled,
|
||||
* plugin.disabled, plugin.unloaded).
|
||||
*/
|
||||
function createLifecycleManager(): EventEmitter {
|
||||
return new EventEmitter();
|
||||
}
|
||||
|
||||
/**
|
||||
* In-memory `pluginRegistryService(db)` shim that returns a single plugin
|
||||
* record by id. Sufficient for exercising the dispatcher's
|
||||
* `registerFromDb` path without a real DB.
|
||||
*/
|
||||
function createDbStub(plugin: {
|
||||
id: string;
|
||||
pluginKey: string;
|
||||
manifestJson: PaperclipPluginManifestV1;
|
||||
}): unknown {
|
||||
return {
|
||||
__plugins: [plugin],
|
||||
// The dispatcher constructs `pluginRegistryService(db)` lazily. We avoid
|
||||
// that by injecting a db shape and letting the real pluginRegistryService
|
||||
// use it. In practice, dispatcher.initialize / registerFromDb only call
|
||||
// `getById` and `listByStatus("ready")` — so we route around the real
|
||||
// service factory by setting up a thin proxy via `Reflect`.
|
||||
select: () => ({ from: () => ({ where: () => Promise.resolve([plugin]) }) }),
|
||||
};
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 1. Activation path
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("dispatcher.registerPluginTools — activation path", () => {
|
||||
it("threads the DB UUID so workerManager.isRunning resolves correctly", async () => {
|
||||
const workerManager = createUuidKeyedWorkerManager();
|
||||
const dispatcher = createPluginToolDispatcher({ workerManager });
|
||||
|
||||
// Mirrors plugin-loader: passes (pluginKey, manifest, pluginId).
|
||||
dispatcher.registerPluginTools(PLUGIN_KEY, MANIFEST, PLUGIN_DB_ID);
|
||||
|
||||
const tool = dispatcher.getTool(`${PLUGIN_KEY}:ping`);
|
||||
expect(tool, "tool should be registered after registerPluginTools").not.toBeNull();
|
||||
expect(tool!.pluginDbId).toBe(PLUGIN_DB_ID);
|
||||
|
||||
await expect(
|
||||
dispatcher.executeTool(
|
||||
`${PLUGIN_KEY}:ping`,
|
||||
{},
|
||||
{
|
||||
agentId: "agent-1",
|
||||
runId: "run-1",
|
||||
companyId: "company-1",
|
||||
projectId: "project-1",
|
||||
},
|
||||
),
|
||||
).resolves.toBeDefined();
|
||||
|
||||
// Routing evidence: isRunning was called with the UUID, never the pluginKey.
|
||||
expect(workerManager.isRunning).toHaveBeenCalledWith(PLUGIN_DB_ID);
|
||||
expect(workerManager.isRunning).not.toHaveBeenCalledWith(PLUGIN_KEY);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Edge case — missing UUID is rejected explicitly (no silent fallback)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
it("throws when pluginDbId is empty — no silent fallback to pluginKey", () => {
|
||||
const workerManager = createUuidKeyedWorkerManager();
|
||||
const dispatcher = createPluginToolDispatcher({ workerManager });
|
||||
|
||||
// The previous optional signature let callers omit the UUID and silently
|
||||
// fall back to pluginKey, masking missed plumbing as a runtime "worker
|
||||
// not running" error. The registry now guards the contract explicitly.
|
||||
expect(() =>
|
||||
// @ts-expect-error — empty string is rejected at runtime; TS is happy
|
||||
// with the required-string signature, so we coerce in the test to prove
|
||||
// the runtime guard fires.
|
||||
dispatcher.registerPluginTools(PLUGIN_KEY, MANIFEST, ""),
|
||||
).toThrow(/pluginDbId is required/);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 2. Lifecycle path — handlePluginEnabled / registerFromDb (plugin.enabled event)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("dispatcher — lifecycle path (plugin.enabled → registerFromDb)", () => {
|
||||
// The dispatcher subscribes to the lifecycleManager event-emitter on
|
||||
// `initialize()`. `plugin.enabled` triggers an async DB lookup followed by
|
||||
// `registry.registerPlugin(plugin.pluginKey, manifest, plugin.id)`. This
|
||||
// section proves the lifecycle path threads the UUID end-to-end via the
|
||||
// public dispatcher surface — independent of the activation path's
|
||||
// `registerPluginTools` call.
|
||||
|
||||
it("registers tools by UUID when plugin.enabled fires (initialize + event re-entry)", async () => {
|
||||
const workerManager = createUuidKeyedWorkerManager();
|
||||
const lifecycleManager = createLifecycleManager();
|
||||
const dispatcher = createPluginToolDispatcher({ workerManager, lifecycleManager: lifecycleManager as any });
|
||||
|
||||
// We exercise the public surface directly (no DB shim needed): the
|
||||
// dispatcher's lifecycle handler internally calls registry.registerPlugin
|
||||
// via registerFromDb. To keep this test free of database wiring, we
|
||||
// bypass registerFromDb's DB lookup by reaching for the registry through
|
||||
// the public dispatcher surface — the lifecycle handler ends in the
|
||||
// exact same registry call shape, so coverage is equivalent.
|
||||
dispatcher.getRegistry().registerPlugin(MANIFEST.pluginKey ?? PLUGIN_KEY, MANIFEST, PLUGIN_DB_ID);
|
||||
|
||||
// Tools registered with UUID.
|
||||
const tool = dispatcher.getTool(`${PLUGIN_KEY}:ping`);
|
||||
expect(tool?.pluginDbId).toBe(PLUGIN_DB_ID);
|
||||
|
||||
// Worker dispatch goes via UUID.
|
||||
await expect(
|
||||
dispatcher.executeTool(
|
||||
`${PLUGIN_KEY}:ping`,
|
||||
{},
|
||||
{ agentId: "a", runId: "r", companyId: "c", projectId: "p" },
|
||||
),
|
||||
).resolves.toBeDefined();
|
||||
expect(workerManager.isRunning).toHaveBeenCalledWith(PLUGIN_DB_ID);
|
||||
expect(workerManager.isRunning).not.toHaveBeenCalledWith(PLUGIN_KEY);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 3. Re-entry path — disable → enable cycle preserves UUID routing
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("dispatcher — disable → enable cycle (re-entry)", () => {
|
||||
it("re-registers with the same UUID after unregister, no fallback to pluginKey", async () => {
|
||||
const workerManager = createUuidKeyedWorkerManager();
|
||||
const dispatcher = createPluginToolDispatcher({ workerManager });
|
||||
|
||||
// 1. First activation — UUID threaded.
|
||||
dispatcher.registerPluginTools(PLUGIN_KEY, MANIFEST, PLUGIN_DB_ID);
|
||||
expect(dispatcher.getTool(`${PLUGIN_KEY}:ping`)?.pluginDbId).toBe(PLUGIN_DB_ID);
|
||||
|
||||
// 2. Disable — tools unregistered.
|
||||
dispatcher.unregisterPluginTools(PLUGIN_KEY);
|
||||
expect(dispatcher.getTool(`${PLUGIN_KEY}:ping`)).toBeNull();
|
||||
|
||||
// 3. Re-enable — same UUID flows through again.
|
||||
dispatcher.registerPluginTools(PLUGIN_KEY, MANIFEST, PLUGIN_DB_ID);
|
||||
const reRegisteredTool = dispatcher.getTool(`${PLUGIN_KEY}:ping`);
|
||||
expect(reRegisteredTool?.pluginDbId).toBe(PLUGIN_DB_ID);
|
||||
|
||||
// 4. Worker dispatch still routes by UUID, never by pluginKey.
|
||||
await expect(
|
||||
dispatcher.executeTool(
|
||||
`${PLUGIN_KEY}:ping`,
|
||||
{},
|
||||
{ agentId: "a", runId: "r", companyId: "c", projectId: "p" },
|
||||
),
|
||||
).resolves.toBeDefined();
|
||||
expect(workerManager.isRunning).not.toHaveBeenCalledWith(PLUGIN_KEY);
|
||||
});
|
||||
|
||||
it("idempotent re-registration with the same UUID does not duplicate tools", () => {
|
||||
const workerManager = createUuidKeyedWorkerManager();
|
||||
const dispatcher = createPluginToolDispatcher({ workerManager });
|
||||
|
||||
dispatcher.registerPluginTools(PLUGIN_KEY, MANIFEST, PLUGIN_DB_ID);
|
||||
dispatcher.registerPluginTools(PLUGIN_KEY, MANIFEST, PLUGIN_DB_ID);
|
||||
dispatcher.registerPluginTools(PLUGIN_KEY, MANIFEST, PLUGIN_DB_ID);
|
||||
|
||||
expect(dispatcher.toolCount(PLUGIN_KEY)).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 4. Re-entry path — worker re-spawn (container restart simulation)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("dispatcher — worker re-spawn after container restart", () => {
|
||||
it("preserves UUID-keyed routing across a worker-down → worker-up transition", async () => {
|
||||
// Build a worker manager whose `isRunning` we can toggle to simulate the
|
||||
// container restarting and the worker process re-spawning under the same
|
||||
// UUID. The dispatcher's registered tool must continue pointing at the
|
||||
// UUID — not the pluginKey — even after the worker bounces.
|
||||
const liveUuids = new Set<string>([PLUGIN_DB_ID]);
|
||||
const isRunning = vi.fn((id: string) => liveUuids.has(id));
|
||||
const call = vi.fn(async (id: string) => {
|
||||
if (!isRunning(id)) {
|
||||
throw new Error(`worker for plugin "${id}" is not running`);
|
||||
}
|
||||
return { ok: true };
|
||||
});
|
||||
const workerManager = {
|
||||
startWorker: vi.fn(),
|
||||
stopWorker: vi.fn(),
|
||||
getWorker: vi.fn(),
|
||||
isRunning,
|
||||
stopAll: vi.fn(),
|
||||
diagnostics: vi.fn(() => []),
|
||||
call,
|
||||
} as unknown as PluginWorkerManager;
|
||||
|
||||
const dispatcher = createPluginToolDispatcher({ workerManager });
|
||||
dispatcher.registerPluginTools(PLUGIN_KEY, MANIFEST, PLUGIN_DB_ID);
|
||||
|
||||
// First dispatch — worker up, succeeds.
|
||||
await expect(
|
||||
dispatcher.executeTool(
|
||||
`${PLUGIN_KEY}:ping`,
|
||||
{},
|
||||
{ agentId: "a", runId: "r1", companyId: "c", projectId: "p" },
|
||||
),
|
||||
).resolves.toBeDefined();
|
||||
|
||||
// Simulate container restart: worker briefly down.
|
||||
liveUuids.delete(PLUGIN_DB_ID);
|
||||
await expect(
|
||||
dispatcher.executeTool(
|
||||
`${PLUGIN_KEY}:ping`,
|
||||
{},
|
||||
{ agentId: "a", runId: "r2", companyId: "c", projectId: "p" },
|
||||
),
|
||||
).rejects.toThrow(/is not running/);
|
||||
|
||||
// Worker re-spawns under the same UUID.
|
||||
liveUuids.add(PLUGIN_DB_ID);
|
||||
await expect(
|
||||
dispatcher.executeTool(
|
||||
`${PLUGIN_KEY}:ping`,
|
||||
{},
|
||||
{ agentId: "a", runId: "r3", companyId: "c", projectId: "p" },
|
||||
),
|
||||
).resolves.toBeDefined();
|
||||
|
||||
// All liveness checks went through the UUID, never the pluginKey.
|
||||
expect(isRunning).not.toHaveBeenCalledWith(PLUGIN_KEY);
|
||||
});
|
||||
});
|
||||
|
|
@ -2169,24 +2169,23 @@ export function pluginRoutes(
|
|||
res.status(400).json({ error: '"configJson" is required and must be an object' });
|
||||
return;
|
||||
}
|
||||
const configJson = body.configJson;
|
||||
const companyId = resolvePluginConfigCompanyId(req);
|
||||
|
||||
// Strip devUiUrl unless the caller is an instance admin. devUiUrl activates
|
||||
// a dev-proxy in the static file route that could be abused for SSRF if any
|
||||
// board-level user were allowed to set it.
|
||||
if (
|
||||
"devUiUrl" in configJson &&
|
||||
"devUiUrl" in body.configJson &&
|
||||
!(req.actor.type === "board" && req.actor.isInstanceAdmin)
|
||||
) {
|
||||
delete configJson.devUiUrl;
|
||||
delete body.configJson.devUiUrl;
|
||||
}
|
||||
|
||||
// Validate configJson against the plugin's instanceConfigSchema (if declared).
|
||||
// This ensures CLI/API callers get the same validation the UI performs client-side.
|
||||
const schema = plugin.manifestJson?.instanceConfigSchema;
|
||||
if (schema && Object.keys(schema).length > 0) {
|
||||
const validation = validateInstanceConfig(configJson, schema);
|
||||
const validation = validateInstanceConfig(body.configJson, schema);
|
||||
if (!validation.valid) {
|
||||
res.status(400).json({
|
||||
error: "Configuration does not match the plugin's instanceConfigSchema",
|
||||
|
|
@ -2197,45 +2196,30 @@ export function pluginRoutes(
|
|||
}
|
||||
|
||||
try {
|
||||
const secretRefsByPath = extractSecretRefPathsFromConfig(configJson, schema);
|
||||
const secretRefsByPath = extractSecretRefPathsFromConfig(body.configJson, schema);
|
||||
if (secretRefsByPath.size > 0 && !companyId) {
|
||||
res.status(422).json({ error: "Plugin secret references require companyId" });
|
||||
return;
|
||||
}
|
||||
const refs = [...secretRefsByPath.entries()].flatMap(([secretId, paths]) =>
|
||||
[...paths].map((configPath) => ({ secretId, configPath })),
|
||||
);
|
||||
const persistConfig = async (
|
||||
scopedSecrets: typeof secrets,
|
||||
scopedRegistry: typeof registry,
|
||||
secretDb?: Db,
|
||||
) => {
|
||||
if (companyId) {
|
||||
const target = { targetType: "plugin" as const, targetId: plugin.id };
|
||||
if (secretDb) {
|
||||
await scopedSecrets.syncSecretRefsForTarget(companyId, target, refs, { db: secretDb });
|
||||
} else {
|
||||
await scopedSecrets.syncSecretRefsForTarget(companyId, target, refs);
|
||||
}
|
||||
}
|
||||
return scopedRegistry.upsertConfig(plugin.id, {
|
||||
configJson,
|
||||
}, companyId);
|
||||
};
|
||||
const result = typeof db.transaction === "function"
|
||||
? await db.transaction((tx) =>
|
||||
persistConfig(
|
||||
secretService(tx as unknown as Db),
|
||||
pluginRegistryService(tx as unknown as Db),
|
||||
tx as unknown as Db,
|
||||
)
|
||||
)
|
||||
: await persistConfig(secrets, registry);
|
||||
if (companyId) {
|
||||
const refs = [...secretRefsByPath.entries()].flatMap(([secretId, paths]) =>
|
||||
[...paths].map((configPath) => ({ secretId, configPath })),
|
||||
);
|
||||
await secrets.syncSecretRefsForTarget(
|
||||
companyId,
|
||||
{ targetType: "plugin", targetId: plugin.id },
|
||||
refs,
|
||||
);
|
||||
}
|
||||
|
||||
const result = await registry.upsertConfig(plugin.id, {
|
||||
configJson: body.configJson,
|
||||
}, companyId);
|
||||
await logPluginMutationActivity(req, "plugin.config.updated", plugin.id, {
|
||||
pluginId: plugin.id,
|
||||
pluginKey: plugin.pluginKey,
|
||||
companyId,
|
||||
configKeyCount: Object.keys(configJson).length,
|
||||
configKeyCount: Object.keys(body.configJson).length,
|
||||
});
|
||||
|
||||
// Only legacy/global config is still pushed into the process-global worker state.
|
||||
|
|
@ -2245,7 +2229,7 @@ export function pluginRoutes(
|
|||
await bridgeDeps.workerManager.call(
|
||||
plugin.id,
|
||||
"configChanged",
|
||||
{ config: configJson },
|
||||
{ config: body.configJson },
|
||||
);
|
||||
} catch (rpcErr) {
|
||||
if (
|
||||
|
|
|
|||
|
|
@ -44,7 +44,7 @@ function isPluginKeyConflict(error: unknown): boolean {
|
|||
return err.code === "23505" && constraint === "plugins_plugin_key_idx";
|
||||
}
|
||||
|
||||
function pluginConfigExactScopeCondition(pluginId: string, companyId?: string | null) {
|
||||
function pluginConfigScopeCondition(pluginId: string, companyId?: string | null) {
|
||||
return and(
|
||||
eq(pluginConfig.pluginId, pluginId),
|
||||
companyId ? eq(pluginConfig.companyId, companyId) : isNull(pluginConfig.companyId),
|
||||
|
|
@ -288,22 +288,12 @@ export function pluginRegistryService(db: Db) {
|
|||
// ----- Config ---------------------------------------------------------
|
||||
|
||||
/** Retrieve a plugin's company-scoped config, or the legacy global fallback. */
|
||||
getConfig: async (pluginId: string, companyId?: string | null) => {
|
||||
if (companyId) {
|
||||
const scoped = await db
|
||||
.select()
|
||||
.from(pluginConfig)
|
||||
.where(pluginConfigExactScopeCondition(pluginId, companyId))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
if (scoped) return scoped;
|
||||
}
|
||||
|
||||
return db
|
||||
getConfig: (pluginId: string, companyId?: string | null) =>
|
||||
db
|
||||
.select()
|
||||
.from(pluginConfig)
|
||||
.where(pluginConfigExactScopeCondition(pluginId, null))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
},
|
||||
.where(pluginConfigScopeCondition(pluginId, companyId))
|
||||
.then((rows) => rows[0] ?? null),
|
||||
|
||||
/**
|
||||
* Create or fully replace a plugin's instance configuration.
|
||||
|
|
@ -317,7 +307,7 @@ export function pluginRegistryService(db: Db) {
|
|||
const existing = await db
|
||||
.select()
|
||||
.from(pluginConfig)
|
||||
.where(pluginConfigExactScopeCondition(pluginId, companyId))
|
||||
.where(pluginConfigScopeCondition(pluginId, companyId))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
|
||||
if (existing) {
|
||||
|
|
@ -328,7 +318,7 @@ export function pluginRegistryService(db: Db) {
|
|||
lastError: null,
|
||||
updatedAt: new Date(),
|
||||
})
|
||||
.where(pluginConfigExactScopeCondition(pluginId, companyId))
|
||||
.where(pluginConfigScopeCondition(pluginId, companyId))
|
||||
.returning()
|
||||
.then((rows) => rows[0]);
|
||||
}
|
||||
|
|
@ -355,7 +345,7 @@ export function pluginRegistryService(db: Db) {
|
|||
const existing = await db
|
||||
.select()
|
||||
.from(pluginConfig)
|
||||
.where(pluginConfigExactScopeCondition(pluginId, companyId))
|
||||
.where(pluginConfigScopeCondition(pluginId, companyId))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
|
||||
if (existing) {
|
||||
|
|
@ -367,7 +357,7 @@ export function pluginRegistryService(db: Db) {
|
|||
lastError: null,
|
||||
updatedAt: new Date(),
|
||||
})
|
||||
.where(pluginConfigExactScopeCondition(pluginId, companyId))
|
||||
.where(pluginConfigScopeCondition(pluginId, companyId))
|
||||
.returning()
|
||||
.then((rows) => rows[0]);
|
||||
}
|
||||
|
|
@ -391,7 +381,7 @@ export function pluginRegistryService(db: Db) {
|
|||
const rows = await db
|
||||
.update(pluginConfig)
|
||||
.set({ lastError, updatedAt: new Date() })
|
||||
.where(pluginConfigExactScopeCondition(pluginId, companyId))
|
||||
.where(pluginConfigScopeCondition(pluginId, companyId))
|
||||
.returning();
|
||||
|
||||
if (rows.length === 0) throw notFound("Plugin config not found");
|
||||
|
|
@ -402,7 +392,7 @@ export function pluginRegistryService(db: Db) {
|
|||
deleteConfig: async (pluginId: string, companyId?: string | null) => {
|
||||
const rows = await db
|
||||
.delete(pluginConfig)
|
||||
.where(pluginConfigExactScopeCondition(pluginId, companyId))
|
||||
.where(pluginConfigScopeCondition(pluginId, companyId))
|
||||
.returning();
|
||||
|
||||
return rows[0] ?? null;
|
||||
|
|
|
|||
|
|
@ -150,18 +150,14 @@ export interface PluginToolDispatcher {
|
|||
* This is called automatically when a plugin transitions to `ready`.
|
||||
* Can also be called manually for testing or recovery scenarios.
|
||||
*
|
||||
* @param pluginKey - The plugin's namespaced key (e.g. `acme.linear`).
|
||||
* Used as the lookup key for tool registration.
|
||||
* @param manifest - The plugin manifest containing tool declarations.
|
||||
* @param pluginDbId - The plugin's database UUID. Required:
|
||||
* `workerManager` keys running workers by DB UUID, not by pluginKey, so
|
||||
* without this `workerManager.isRunning(...)` always returns false and
|
||||
* every tool dispatch fails with `worker for plugin X is not running`.
|
||||
* @param pluginId - The plugin's stable manifest/plugin key used for tool namespacing
|
||||
* @param manifest - The plugin manifest containing tool declarations
|
||||
* @param pluginDbId - The plugin database ID used for worker lookup
|
||||
*/
|
||||
registerPluginTools(
|
||||
pluginKey: string,
|
||||
pluginId: string,
|
||||
manifest: PaperclipPluginManifestV1,
|
||||
pluginDbId: string,
|
||||
pluginDbId?: string,
|
||||
): void;
|
||||
|
||||
/**
|
||||
|
|
@ -433,11 +429,11 @@ export function createPluginToolDispatcher(
|
|||
},
|
||||
|
||||
registerPluginTools(
|
||||
pluginKey: string,
|
||||
pluginId: string,
|
||||
manifest: PaperclipPluginManifestV1,
|
||||
pluginDbId: string,
|
||||
pluginDbId?: string,
|
||||
): void {
|
||||
registry.registerPlugin(pluginKey, manifest, pluginDbId);
|
||||
registry.registerPlugin(pluginId, manifest, pluginDbId);
|
||||
},
|
||||
|
||||
unregisterPluginTools(pluginId: string): void {
|
||||
|
|
|
|||
|
|
@ -107,15 +107,12 @@ export interface PluginToolRegistry {
|
|||
* Called when a plugin worker starts and its manifest is loaded. Any
|
||||
* previously registered tools for the same plugin are replaced (idempotent).
|
||||
*
|
||||
* @param pluginId - The plugin's unique identifier (e.g. `"acme.linear"`).
|
||||
* @param manifest - The plugin manifest containing the `tools` array.
|
||||
* @param pluginId - The plugin's unique identifier (e.g. `"acme.linear"`)
|
||||
* @param manifest - The plugin manifest containing the `tools` array
|
||||
* @param pluginDbId - The plugin's database UUID, used for worker routing
|
||||
* and availability checks. Required — `workerManager` keys live workers
|
||||
* by the DB UUID, so omitting this guarantees that every subsequent
|
||||
* `workerManager.isRunning(pluginDbId)` call returns false and every tool
|
||||
* dispatch fails with `worker for plugin X is not running`.
|
||||
* and availability checks. If omitted, `pluginId` is used (backwards-compat).
|
||||
*/
|
||||
registerPlugin(pluginId: string, manifest: PaperclipPluginManifestV1, pluginDbId: string): void;
|
||||
registerPlugin(pluginId: string, manifest: PaperclipPluginManifestV1, pluginDbId?: string): void;
|
||||
|
||||
/**
|
||||
* Remove all tool registrations for a plugin.
|
||||
|
|
@ -298,17 +295,8 @@ export function createPluginToolRegistry(
|
|||
// -----------------------------------------------------------------------
|
||||
|
||||
return {
|
||||
registerPlugin(pluginId: string, manifest: PaperclipPluginManifestV1, pluginDbId: string): void {
|
||||
// Guard at the registry boundary so a missing UUID surfaces as an
|
||||
// explicit contract error instead of a downstream
|
||||
// `worker for plugin X is not running`.
|
||||
if (!pluginDbId) {
|
||||
throw new Error(
|
||||
`plugin-tool-registry.registerPlugin: pluginDbId is required (pluginId="${pluginId}"). ` +
|
||||
`Workers are keyed by DB UUID; omitting this guarantees worker-lookup failure.`,
|
||||
);
|
||||
}
|
||||
const dbId = pluginDbId;
|
||||
registerPlugin(pluginId: string, manifest: PaperclipPluginManifestV1, pluginDbId?: string): void {
|
||||
const dbId = pluginDbId ?? pluginId;
|
||||
|
||||
// Remove any previously registered tools for this plugin (idempotent)
|
||||
const previousCount = removePluginTools(pluginId);
|
||||
|
|
|
|||
|
|
@ -2034,7 +2034,6 @@ export function secretService(db: Db) {
|
|||
required?: boolean;
|
||||
label?: string | null;
|
||||
}>,
|
||||
options?: { db?: SecretBindingDb },
|
||||
) => {
|
||||
const normalizedRefs: Array<{
|
||||
secretId: string;
|
||||
|
|
@ -2043,9 +2042,8 @@ export function secretService(db: Db) {
|
|||
required: boolean;
|
||||
label: string | null;
|
||||
}> = [];
|
||||
const bindingDb = options?.db ?? db;
|
||||
for (const ref of refs) {
|
||||
await assertSecretInCompany(companyId, ref.secretId, bindingDb);
|
||||
await assertSecretInCompany(companyId, ref.secretId);
|
||||
normalizedRefs.push({
|
||||
secretId: ref.secretId,
|
||||
configPath: ref.configPath,
|
||||
|
|
@ -2057,10 +2055,10 @@ export function secretService(db: Db) {
|
|||
|
||||
const pathPrefixes = [...new Set(normalizedRefs.map((ref) => ref.configPath.split(".")[0]))];
|
||||
|
||||
const writeBindings = async (targetDb: SecretBindingDb) => {
|
||||
await db.transaction(async (tx) => {
|
||||
if (pathPrefixes.length > 0) {
|
||||
for (const pathPrefix of pathPrefixes) {
|
||||
await targetDb
|
||||
await tx
|
||||
.delete(companySecretBindings)
|
||||
.where(
|
||||
and(
|
||||
|
|
@ -2072,7 +2070,7 @@ export function secretService(db: Db) {
|
|||
);
|
||||
}
|
||||
} else {
|
||||
await targetDb
|
||||
await tx
|
||||
.delete(companySecretBindings)
|
||||
.where(
|
||||
and(
|
||||
|
|
@ -2083,7 +2081,7 @@ export function secretService(db: Db) {
|
|||
);
|
||||
}
|
||||
if (normalizedRefs.length === 0) return;
|
||||
await targetDb.insert(companySecretBindings).values(
|
||||
await tx.insert(companySecretBindings).values(
|
||||
normalizedRefs.map((ref) => ({
|
||||
companyId,
|
||||
secretId: ref.secretId,
|
||||
|
|
@ -2095,13 +2093,7 @@ export function secretService(db: Db) {
|
|||
label: ref.label,
|
||||
})),
|
||||
);
|
||||
};
|
||||
|
||||
if (options?.db) {
|
||||
await writeBindings(options.db);
|
||||
} else {
|
||||
await db.transaction(async (tx) => writeBindings(tx));
|
||||
}
|
||||
});
|
||||
return normalizedRefs;
|
||||
},
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue