mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-01 23:43:16 +01:00
Integrate current master before rich ACP foundation review
Co-Authored-By: Paperclip <noreply@paperclip.ing> * commit '53aad90b9': fix: retry sandbox ACP input delivery after gateway failures (#14485) fix: retain project defaults in partial workspace overrides (#14502) test: control Telegram subscription retry timing (#14501) fix: retain diagnostic reasons for native runner failures (#14481) fix: prevent run identity locks from blocking audit checks (#14478) fix: preserve company context across browser hot reload (#14482) feat(ui): improve task composer controls and pending input (#14322) Improve task artifacts with rich cards and editable stories (#14469)
This commit is contained in:
commit
9774086a73
111 files changed
+7338
-1164
No files matched your search
@@ -5,7 +5,7 @@ import { models as codexFallbackModels } from "@paperclipai/adapter-codex-local"
|
||||
import { models as cursorFallbackModels } from "@paperclipai/adapter-cursor-local";
|
||||
import { models as opencodeFallbackModels } from "@paperclipai/adapter-opencode-local";
|
||||
import { resetOpenCodeModelsCacheForTests } from "@paperclipai/adapter-opencode-local/server";
|
||||
import { listAdapterModels, listServerAdapters, refreshAdapterModels } from "../adapters/index.js";
|
||||
import { listAdapterModels, listServerAdapters, refreshAdapterModels, registerServerAdapter, unregisterServerAdapter } from "../adapters/index.js";
|
||||
import { resetCodexModelsCacheForTests } from "../adapters/codex-models.js";
|
||||
import { resetCursorModelsCacheForTests, setCursorModelsRunnerForTests } from "../adapters/cursor-models.js";
|
||||
|
||||
@@ -195,14 +195,14 @@ describe("adapter model listing", () => {
|
||||
]));
|
||||
});
|
||||
|
||||
it("loads codex models dynamically and merges fallback options", async () => {
|
||||
it("keeps general OpenAI API models out of the Codex catalog", async () => {
|
||||
process.env.OPENAI_API_KEY = "sk-test";
|
||||
const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue({
|
||||
ok: true,
|
||||
json: async () => ({
|
||||
data: [
|
||||
{ id: "gpt-5-pro" },
|
||||
{ id: "gpt-5" },
|
||||
{ id: "gpt-image-1" },
|
||||
{ id: "text-embedding-3-large" },
|
||||
],
|
||||
}),
|
||||
} as Response);
|
||||
@@ -210,40 +210,26 @@ describe("adapter model listing", () => {
|
||||
const first = await listAdapterModels("codex_local");
|
||||
const second = await listAdapterModels("codex_local");
|
||||
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(1);
|
||||
expect(first).toEqual(second);
|
||||
expect(first.some((model) => model.id === "gpt-5-pro")).toBe(true);
|
||||
expect(first.some((model) => model.id === "codex-mini-latest")).toBe(true);
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
expect(first).toEqual(codexFallbackModels);
|
||||
expect(second).toEqual(codexFallbackModels);
|
||||
});
|
||||
|
||||
it("refreshes cached codex models on demand", async () => {
|
||||
it("keeps the curated Codex list when refreshing with an OpenAI key", async () => {
|
||||
process.env.OPENAI_API_KEY = "sk-test";
|
||||
const fetchSpy = vi.spyOn(globalThis, "fetch")
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: async () => ({
|
||||
data: [{ id: "gpt-5" }],
|
||||
}),
|
||||
} as Response)
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: async () => ({
|
||||
data: [{ id: "gpt-5.6-terra" }],
|
||||
}),
|
||||
} as Response);
|
||||
const fetchSpy = vi.spyOn(globalThis, "fetch");
|
||||
|
||||
const initial = await listAdapterModels("codex_local");
|
||||
const refreshed = await refreshAdapterModels("codex_local");
|
||||
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(2);
|
||||
expect(initial.some((model) => model.id === "gpt-5")).toBe(true);
|
||||
expect(refreshed.some((model) => model.id === "gpt-5.6-terra")).toBe(true);
|
||||
expect(refreshed.some((model) => model.id === "gpt-5.6-luna")).toBe(true);
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
expect(initial).toEqual(codexFallbackModels);
|
||||
expect(refreshed).toEqual(codexFallbackModels);
|
||||
});
|
||||
|
||||
it("falls back to static codex models when OpenAI model discovery fails", async () => {
|
||||
it("uses static Codex models without calling OpenAI model discovery", async () => {
|
||||
process.env.OPENAI_API_KEY = "sk-test";
|
||||
vi.spyOn(globalThis, "fetch").mockResolvedValue({
|
||||
const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue({
|
||||
ok: false,
|
||||
status: 401,
|
||||
json: async () => ({}),
|
||||
@@ -251,6 +237,33 @@ describe("adapter model listing", () => {
|
||||
|
||||
const models = await listAdapterModels("codex_local");
|
||||
expect(models).toEqual(codexFallbackModels);
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("uses a custom Codex adapter's model and refresh hooks", async () => {
|
||||
const builtin = listServerAdapters().find((adapter) => adapter.type === "codex_local")!;
|
||||
const customModels = [{ id: "plugin-codex", label: "Plugin Codex" }];
|
||||
const listModels = vi.fn(async () => customModels);
|
||||
const refreshModels = vi.fn(async () => customModels);
|
||||
registerServerAdapter({ ...builtin, models: [], listModels, refreshModels });
|
||||
try {
|
||||
await expect(listAdapterModels("codex_local")).resolves.toEqual(customModels);
|
||||
await expect(refreshAdapterModels("codex_local")).resolves.toEqual(customModels);
|
||||
expect(listModels).toHaveBeenCalledOnce();
|
||||
expect(refreshModels).toHaveBeenCalledOnce();
|
||||
|
||||
process.env.PAPERCLIP_ADAPTER_MODELS = JSON.stringify({
|
||||
codex_local: [{ id: "declared-codex", label: "Declared Codex" }],
|
||||
});
|
||||
const declared = [{ id: "declared-codex", label: "Declared Codex" }];
|
||||
await expect(listAdapterModels("codex_local")).resolves.toEqual(declared);
|
||||
await expect(refreshAdapterModels("codex_local")).resolves.toEqual(declared);
|
||||
expect(listModels).toHaveBeenCalledOnce();
|
||||
expect(refreshModels).toHaveBeenCalledOnce();
|
||||
} finally {
|
||||
delete process.env.PAPERCLIP_ADAPTER_MODELS;
|
||||
unregisterServerAdapter("codex_local");
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -315,6 +328,16 @@ describe("adapter model listing", () => {
|
||||
]);
|
||||
});
|
||||
|
||||
it("uses declared Codex models for both listing and refresh", async () => {
|
||||
process.env.PAPERCLIP_ADAPTER_MODELS = JSON.stringify({
|
||||
codex_local: [{ id: "private-codex", label: "Private Codex" }],
|
||||
});
|
||||
const declared = [{ id: "private-codex", label: "Private Codex" }];
|
||||
|
||||
await expect(listAdapterModels("codex_local")).resolves.toEqual(declared);
|
||||
await expect(refreshAdapterModels("codex_local")).resolves.toEqual(declared);
|
||||
});
|
||||
|
||||
it("observes env changes between calls (memo keyed by raw env value)", async () => {
|
||||
process.env.PAPERCLIP_ADAPTER_MODELS = JSON.stringify({
|
||||
opencode_local: [{ id: "model-a" }],
|
||||
|
||||
@@ -56157,9 +56157,13 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
"leased",
|
||||
"wrong_thread",
|
||||
"source_edited",
|
||||
])(
|
||||
"retries only the original pre-provider Telegram request after exact cleanup: %s",
|
||||
async (mode) => {
|
||||
"different_error",
|
||||
].flatMap((mode) => [
|
||||
"runner_state_identity_mismatch",
|
||||
"runner_state_identity_mismatch: prior_owner_active",
|
||||
].map((errorMessage) => ({ mode, errorMessage }))))(
|
||||
"retries only the original pre-provider Telegram request after exact cleanup: $mode ($errorMessage)",
|
||||
async ({ mode, errorMessage }) => {
|
||||
const context = await committedChatResponseRecoveryFixture("telegram");
|
||||
const providerAccount =
|
||||
mode === "null_account"
|
||||
@@ -56286,7 +56290,9 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
agentId: context.fixture.assignedAgentId,
|
||||
status: "failed",
|
||||
errorCode: "adapter_failed",
|
||||
error: "runner_state_identity_mismatch",
|
||||
error: mode === "different_error"
|
||||
? errorMessage.replace("runner_state_identity_mismatch", "runner_state_identity_mismatch_other")
|
||||
: errorMessage,
|
||||
finishedAt: new Date(),
|
||||
wakeupRequestId: action.id,
|
||||
runtimeMode: "native",
|
||||
@@ -69057,7 +69063,11 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
},
|
||||
);
|
||||
|
||||
it("retries an unknown subscription mutation after restart using freshly observed options, not a recovered confirmation flag", async () => {
|
||||
it("retries an unknown subscription mutation after restart using freshly observed options, not a recovered confirmation flag", async ({ onTestFailed }) => {
|
||||
let phase = "create fixture";
|
||||
onTestFailed(() => {
|
||||
console.error(`Telegram subscription recovery failed during: ${phase}`);
|
||||
});
|
||||
const lane = await draftFixture();
|
||||
try {
|
||||
await db
|
||||
@@ -69074,6 +69084,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
throw new Error("Synthetic unknown subscription response");
|
||||
return undefined;
|
||||
});
|
||||
phase = "first subscription attempt";
|
||||
await lane.processSubscriptionAttempt();
|
||||
const [action] = await db
|
||||
.select()
|
||||
@@ -69089,17 +69100,28 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
status: "failed",
|
||||
result: { retryable: true, providerConfirmed: false },
|
||||
});
|
||||
// Keep the retry pending until the explicit post-restart transition below.
|
||||
// A busy runner can otherwise exhaust the real one-second backoff here.
|
||||
await db
|
||||
.update(chatActions)
|
||||
.set({
|
||||
result: { ...action!.result, retryAt: "2099-01-01T00:00:00.000Z" },
|
||||
})
|
||||
.where(eq(chatActions.id, action!.id));
|
||||
phase = "ordinary publication before restart";
|
||||
expect((await lane.send("unknown-subscription"))?.state).toBe(
|
||||
"published",
|
||||
);
|
||||
expect(lane.requests).toHaveLength(1);
|
||||
expect(lane.requests[0]!.method.endsWith("Draft")).toBe(false);
|
||||
phase = "pending recovery before restart";
|
||||
await lane.context.service.processPendingDeliveries();
|
||||
expect(
|
||||
lane.maintenanceRequests.filter(
|
||||
({ method }) => method === "setWebhook",
|
||||
),
|
||||
).toHaveLength(1);
|
||||
phase = "restart";
|
||||
await lane.restart();
|
||||
lane.setSubscriptionInfo({
|
||||
allowed_updates: ["message", "chat_member"],
|
||||
@@ -69115,10 +69137,12 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
},
|
||||
})
|
||||
.where(eq(chatActions.id, action!.id));
|
||||
phase = "concurrent recovery after restart";
|
||||
await Promise.all([
|
||||
lane.context.service.processPendingDeliveries(),
|
||||
lane.context.service.processPendingDeliveries(),
|
||||
]);
|
||||
phase = "verify recovered subscription";
|
||||
const mutations = lane.maintenanceRequests.filter(
|
||||
({ method }) => method === "setWebhook",
|
||||
);
|
||||
@@ -69148,9 +69172,11 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
|
||||
200,
|
||||
);
|
||||
});
|
||||
phase = "publication after subscription recovery";
|
||||
expect((await lane.send("repaired-after-unknown"))?.state).toBe(
|
||||
"cancelled",
|
||||
);
|
||||
phase = "close fixture";
|
||||
} finally {
|
||||
await lane.close();
|
||||
}
|
||||
|
||||
@@ -1009,7 +1009,11 @@ describeEmbeddedPostgres(
|
||||
{ id: fixture.issueId, companyId: fixture.companyId },
|
||||
{
|
||||
kind: "request_confirmation",
|
||||
payload: { version: 1, prompt: "Proceed with the release?" },
|
||||
payload: {
|
||||
version: 1,
|
||||
prompt: "Proceed with the release?",
|
||||
supersedeOnUserComment: true,
|
||||
},
|
||||
},
|
||||
{ agentId: fixture.agentId },
|
||||
);
|
||||
@@ -1053,6 +1057,7 @@ describeEmbeddedPostgres(
|
||||
kind: "ask_user_questions",
|
||||
payload: {
|
||||
version: 1,
|
||||
supersedeOnUserComment: true,
|
||||
questions: [
|
||||
{
|
||||
id: "priority",
|
||||
|
||||
@@ -300,6 +300,107 @@ describe("execution workspace policy helpers", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("partial issue workspace strategies", () => {
|
||||
const projectStrategy = {
|
||||
type: "git_worktree" as const,
|
||||
baseRef: "origin/main",
|
||||
branchTemplate: "{{issue.identifier}}-{{slug}}",
|
||||
worktreeParentDir: ".paperclip/worktrees",
|
||||
provisionCommand: "true",
|
||||
runtimeProvisionCommand: "npm run setup:runtime",
|
||||
teardownCommand: "npm run teardown",
|
||||
};
|
||||
|
||||
function resolveStrategy(
|
||||
strategy: Record<string, unknown>,
|
||||
enabled = true,
|
||||
) {
|
||||
return buildExecutionWorkspaceAdapterConfig({
|
||||
agentConfig: { workspaceStrategy: { type: "git_worktree", provisionCommand: "agent-setup" } },
|
||||
projectPolicy: parseProjectExecutionWorkspacePolicy({
|
||||
enabled,
|
||||
defaultMode: "isolated_workspace",
|
||||
workspaceStrategy: projectStrategy,
|
||||
}),
|
||||
issueSettings: parseIssueExecutionWorkspaceSettings({
|
||||
mode: "isolated_workspace",
|
||||
workspaceStrategy: strategy,
|
||||
}),
|
||||
mode: "isolated_workspace",
|
||||
legacyUseProjectWorkspace: null,
|
||||
}).workspaceStrategy;
|
||||
}
|
||||
|
||||
it("retains project hooks when an issue changes only its base branch", () => {
|
||||
expect(resolveStrategy({ type: "git_worktree", baseRef: "origin/release" })).toEqual({
|
||||
...projectStrategy,
|
||||
baseRef: "origin/release",
|
||||
});
|
||||
});
|
||||
|
||||
it.each(["npm run issue-setup", "", null])("honors an explicit provisioning override of %j", (provisionCommand) => {
|
||||
expect(resolveStrategy({ type: "git_worktree", provisionCommand })).toEqual({
|
||||
...projectStrategy,
|
||||
provisionCommand,
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves explicit null clears through persisted JSON parsing", () => {
|
||||
const strategy = {
|
||||
type: "git_worktree",
|
||||
baseRef: null,
|
||||
branchTemplate: null,
|
||||
worktreeParentDir: null,
|
||||
provisionCommand: null,
|
||||
runtimeProvisionCommand: null,
|
||||
teardownCommand: null,
|
||||
};
|
||||
expect(resolveStrategy(strategy)).toEqual(strategy);
|
||||
});
|
||||
|
||||
it.each(["cloud_sandbox", "adapter_managed", "project_primary"])("does not carry project hooks into %s", (type) => {
|
||||
expect(resolveStrategy({ type })).toEqual({ type });
|
||||
});
|
||||
|
||||
it("does not inherit a disabled project strategy", () => {
|
||||
expect(resolveStrategy({ type: "git_worktree", baseRef: "origin/release" }, false)).toEqual({
|
||||
type: "git_worktree",
|
||||
baseRef: "origin/release",
|
||||
});
|
||||
expect(resolveStrategy({}, false)).toEqual({
|
||||
type: "git_worktree",
|
||||
provisionCommand: "agent-setup",
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps project hooks for an exact branch pin without inheriting a branch template", () => {
|
||||
const resolved = resolveStrategy({ type: "git_worktree", existingBranch: "fix/existing" });
|
||||
expect(resolved).toEqual({
|
||||
...projectStrategy,
|
||||
branchTemplate: undefined,
|
||||
existingBranch: "fix/existing",
|
||||
});
|
||||
expect(issueExecutionWorkspaceSettingsSchema.safeParse({
|
||||
mode: "isolated_workspace",
|
||||
workspaceStrategy: resolved,
|
||||
}).success).toBe(true);
|
||||
});
|
||||
|
||||
it("does not mutate the project or issue strategy", () => {
|
||||
const issueStrategy = { type: "git_worktree" as const, baseRef: "origin/release" };
|
||||
const result = buildExecutionWorkspaceAdapterConfig({
|
||||
agentConfig: {},
|
||||
projectPolicy: { enabled: true, workspaceStrategy: Object.freeze({ ...projectStrategy }) },
|
||||
issueSettings: { workspaceStrategy: Object.freeze(issueStrategy) },
|
||||
mode: "isolated_workspace",
|
||||
legacyUseProjectWorkspace: null,
|
||||
});
|
||||
expect(result.workspaceStrategy).not.toBe(issueStrategy);
|
||||
expect(issueStrategy).toEqual({ type: "git_worktree", baseRef: "origin/release" });
|
||||
expect(projectStrategy.baseRef).toBe("origin/main");
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves project authorization policy for trust-preset resolution", () => {
|
||||
expect(parseProjectExecutionWorkspacePolicy({
|
||||
enabled: true,
|
||||
|
||||
@@ -394,6 +394,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
continuationPolicy: "wake_assignee",
|
||||
payload: {
|
||||
version: 1,
|
||||
supersedeOnUserComment: true,
|
||||
questions: [{
|
||||
id: "scope",
|
||||
prompt: "Which scope?",
|
||||
@@ -1435,7 +1436,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("expires ask_user_questions interactions by default when a user comments after creation", async () => {
|
||||
it("expires ask_user_questions when a creator opts into comment supersede", async () => {
|
||||
const { companyId, issueId } = await seedConfirmationIssue("Question supersede");
|
||||
const commentId = randomUUID();
|
||||
|
||||
@@ -1446,6 +1447,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
kind: "ask_user_questions",
|
||||
payload: {
|
||||
version: 1,
|
||||
supersedeOnUserComment: true,
|
||||
questions: [{
|
||||
id: "scope",
|
||||
prompt: "Choose the scope",
|
||||
@@ -1491,7 +1493,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps ask_user_questions pending when user-comment supersede is explicitly disabled", async () => {
|
||||
it("keeps ask_user_questions pending by default when the user sends a message", async () => {
|
||||
const { companyId, issueId } = await seedConfirmationIssue("Question supersede opt-out");
|
||||
|
||||
await interactionsSvc.create({
|
||||
@@ -1501,7 +1503,6 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
kind: "ask_user_questions",
|
||||
payload: {
|
||||
version: 1,
|
||||
supersedeOnUserComment: false,
|
||||
questions: [{
|
||||
id: "scope",
|
||||
prompt: "Choose the scope",
|
||||
@@ -1513,6 +1514,9 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
userId: "local-board",
|
||||
});
|
||||
|
||||
const [created] = await db.select().from(issueThreadInteractions);
|
||||
expect(created?.payload).toMatchObject({ supersedeOnUserComment: false });
|
||||
|
||||
const expired = await interactionsSvc.expireRequestConfirmationsSupersededByComment({
|
||||
id: issueId,
|
||||
companyId,
|
||||
@@ -1600,6 +1604,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
kind: "ask_user_questions",
|
||||
payload: {
|
||||
version: 1,
|
||||
supersedeOnUserComment: true,
|
||||
questions: [{
|
||||
id: "scope",
|
||||
prompt: "Choose the scope",
|
||||
@@ -2519,7 +2524,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
status: "pending",
|
||||
continuationPolicy: "wake_assignee",
|
||||
payload: {
|
||||
supersedeOnUserComment: true,
|
||||
supersedeOnUserComment: false,
|
||||
allowDeclineReason: true,
|
||||
},
|
||||
});
|
||||
@@ -2618,6 +2623,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
payload: {
|
||||
version: 1,
|
||||
prompt: "Which files should be deleted?",
|
||||
supersedeOnUserComment: true,
|
||||
options: [{ id: "file-a", label: "a.txt" }],
|
||||
},
|
||||
}, {
|
||||
@@ -2648,6 +2654,27 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps checkbox confirmations pending by default after a user comment", async () => {
|
||||
const { companyId, issueId } = await seedConfirmationIssue("Checkbox card remains");
|
||||
const created = await interactionsSvc.create({ id: issueId, companyId }, {
|
||||
kind: "request_checkbox_confirmation",
|
||||
payload: {
|
||||
version: 1,
|
||||
prompt: "Choose a file",
|
||||
options: [{ id: "file-a", label: "a.txt" }],
|
||||
},
|
||||
}, { userId: "local-board" });
|
||||
expect(created.payload.supersedeOnUserComment).toBe(false);
|
||||
|
||||
const expired = await interactionsSvc.expireRequestConfirmationsSupersededByComment(
|
||||
{ id: issueId, companyId },
|
||||
{ id: randomUUID(), createdAt: new Date(Date.now() + 1_000), authorUserId: "local-board" },
|
||||
{ userId: "local-board" },
|
||||
);
|
||||
expect(expired).toHaveLength(0);
|
||||
expect((await db.select().from(issueThreadInteractions))[0]?.status).toBe("pending");
|
||||
});
|
||||
|
||||
it("submits request_item_verdicts partially and completes when all items are resolved", async () => {
|
||||
const { companyId, issueId } = await seedConfirmationIssue("Item verdict partial submit");
|
||||
|
||||
@@ -2677,7 +2704,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
verdicts: ["approve", "reject"],
|
||||
requireReasonOn: ["reject"],
|
||||
allowBulkApprove: true,
|
||||
supersedeOnUserComment: true,
|
||||
supersedeOnUserComment: false,
|
||||
},
|
||||
});
|
||||
|
||||
@@ -2835,6 +2862,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
payload: {
|
||||
version: 1,
|
||||
prompt: "Review generated artifacts.",
|
||||
supersedeOnUserComment: true,
|
||||
items: [
|
||||
{ id: "api", label: "API route" },
|
||||
{ id: "docs", label: "Docs" },
|
||||
@@ -2885,6 +2913,27 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps item verdict requests pending by default after a user comment", async () => {
|
||||
const { companyId, issueId } = await seedConfirmationIssue("Verdict card remains");
|
||||
const created = await interactionsSvc.create({ id: issueId, companyId }, {
|
||||
kind: "request_item_verdicts",
|
||||
payload: {
|
||||
version: 1,
|
||||
prompt: "Review the file",
|
||||
items: [{ id: "file-a", label: "a.txt" }],
|
||||
},
|
||||
}, { userId: "local-board" });
|
||||
expect(created.payload.supersedeOnUserComment).toBe(false);
|
||||
|
||||
const expired = await interactionsSvc.expireRequestConfirmationsSupersededByComment(
|
||||
{ id: issueId, companyId },
|
||||
{ id: randomUUID(), createdAt: new Date(Date.now() + 1_000), authorUserId: "local-board" },
|
||||
{ userId: "local-board" },
|
||||
);
|
||||
expect(expired).toHaveLength(0);
|
||||
expect((await db.select().from(issueThreadInteractions))[0]?.status).toBe("pending");
|
||||
});
|
||||
|
||||
it("returns accepted agent confirmations from review without resetting active work", async () => {
|
||||
const companyId = randomUUID();
|
||||
const goalId = randomUUID();
|
||||
@@ -3187,7 +3236,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
.resolves.toBe("planning");
|
||||
});
|
||||
|
||||
it("expires request confirmations by default when a user comments after creation", async () => {
|
||||
it("expires request confirmations when a creator opts into comment supersede", async () => {
|
||||
const { companyId, issueId } = await seedConfirmationIssue();
|
||||
const commentId = randomUUID();
|
||||
|
||||
@@ -3199,6 +3248,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
payload: {
|
||||
version: 1,
|
||||
prompt: "Proceed with the current draft?",
|
||||
supersedeOnUserComment: true,
|
||||
},
|
||||
}, {
|
||||
userId: "local-board",
|
||||
@@ -3234,7 +3284,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps request confirmations pending when user-comment supersede is explicitly disabled", async () => {
|
||||
it("keeps request confirmations pending by default when the user sends a message", async () => {
|
||||
const { companyId, issueId } = await seedConfirmationIssue("Comment supersede opt-out");
|
||||
|
||||
await interactionsSvc.create({
|
||||
@@ -3245,12 +3295,14 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
payload: {
|
||||
version: 1,
|
||||
prompt: "Proceed with the current draft?",
|
||||
supersedeOnUserComment: false,
|
||||
},
|
||||
}, {
|
||||
userId: "local-board",
|
||||
});
|
||||
|
||||
const [created] = await db.select().from(issueThreadInteractions);
|
||||
expect(created?.payload).toMatchObject({ supersedeOnUserComment: false });
|
||||
|
||||
const expired = await interactionsSvc.expireRequestConfirmationsSupersededByComment({
|
||||
id: issueId,
|
||||
companyId,
|
||||
@@ -3475,6 +3527,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
|
||||
payload: {
|
||||
version: 1,
|
||||
prompt: "Proceed with the current draft?",
|
||||
supersedeOnUserComment: true,
|
||||
},
|
||||
}, {
|
||||
userId: "local-board",
|
||||
|
||||
@@ -438,6 +438,7 @@ describeEmbeddedPostgres("issueThreadInteractionService telemetry", () => {
|
||||
continuationPolicy: "wake_assignee",
|
||||
payload: {
|
||||
version: 1,
|
||||
supersedeOnUserComment: true,
|
||||
questions: [
|
||||
{
|
||||
id: "scope",
|
||||
@@ -563,6 +564,7 @@ describeEmbeddedPostgres("issueThreadInteractionService telemetry", () => {
|
||||
payload: {
|
||||
version: 1,
|
||||
prompt: "Approve this plan?",
|
||||
supersedeOnUserComment: true,
|
||||
},
|
||||
}, {
|
||||
userId: "local-board",
|
||||
|
||||
@@ -201,7 +201,8 @@ function registerModuleMocks() {
|
||||
}));
|
||||
}
|
||||
|
||||
async function createApp() {
|
||||
async function createApp(transaction: (callback: (tx: Record<string, never>) => Promise<unknown>) => Promise<unknown> =
|
||||
async (callback) => callback({})) {
|
||||
const [{ errorHandler }, { issueRoutes }] = await Promise.all([
|
||||
vi.importActual<typeof import("../middleware/index.js")>("../middleware/index.js"),
|
||||
vi.importActual<typeof import("../routes/issues.js")>("../routes/issues.js"),
|
||||
@@ -220,7 +221,7 @@ async function createApp() {
|
||||
next();
|
||||
});
|
||||
app.use("/api", issueRoutes({
|
||||
transaction: async (callback: (tx: Record<string, never>) => Promise<unknown>) => callback({}),
|
||||
transaction,
|
||||
} as any, {} as any));
|
||||
app.use(errorHandler);
|
||||
return app;
|
||||
@@ -330,6 +331,7 @@ describe("issue update comment wakeups", () => {
|
||||
externalConversationState,
|
||||
assigneeAgentId: ASSIGNEE_AGENT_ID,
|
||||
assigneeUserId: null,
|
||||
assigneeAdapterOverrides: { adapterConfig: { model: "gpt-6-astra", modelReasoningEffort: "ultra", fastMode: true } },
|
||||
});
|
||||
mockIssueService.getById.mockResolvedValue(existing);
|
||||
mockIssueService.update.mockResolvedValue(updated);
|
||||
@@ -347,11 +349,16 @@ describe("issue update comment wakeups", () => {
|
||||
assigneeUserId: null,
|
||||
comment: "write the whole thing",
|
||||
commentClientRequestId: "55555555-5555-4555-8555-555555555555",
|
||||
assigneeAdapterOverrides: updated.assigneeAdapterOverrides,
|
||||
});
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(mockIssueService.update).toHaveBeenCalledWith(existing.id, expect.objectContaining({
|
||||
assigneeAdapterOverrides: updated.assigneeAdapterOverrides,
|
||||
}), expect.anything());
|
||||
expect(mockIssueService.addComment).toHaveBeenCalledWith(existing.id, "write the whole thing", expect.anything(),
|
||||
expect.objectContaining({ clientRequestId: "55555555-5555-4555-8555-555555555555" }));
|
||||
expect.objectContaining({ clientRequestId: "55555555-5555-4555-8555-555555555555" }), expect.anything());
|
||||
expect(mockIssueService.update.mock.calls[0]?.[2]).toBe(mockIssueService.addComment.mock.calls[0]?.[4]);
|
||||
// The route dispatches the wake after it sends the response, so wait for
|
||||
// the fire-and-forget dispatch to settle. This keeps the wake inside this
|
||||
// test and stops it from leaking into the next test as an extra call.
|
||||
@@ -378,6 +385,35 @@ describe("issue update comment wakeups", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("rolls back adapter settings if the accompanying comment fails", async () => {
|
||||
const existing = makeIssue({ assigneeAgentId: ASSIGNEE_AGENT_ID, assigneeUserId: null });
|
||||
let persistedModel = "gpt-6-sol";
|
||||
mockIssueService.getById.mockResolvedValue(existing);
|
||||
mockIssueService.update.mockImplementation(async (_id, fields) => {
|
||||
persistedModel = fields.assigneeAdapterOverrides.adapterConfig.model;
|
||||
return { ...existing, ...fields };
|
||||
});
|
||||
mockIssueService.addComment.mockRejectedValue(new Error("comment write failed"));
|
||||
const transaction = vi.fn(async (callback: (tx: Record<string, never>) => Promise<unknown>) => {
|
||||
const previousModel = persistedModel;
|
||||
try {
|
||||
return await callback({});
|
||||
} catch (error) {
|
||||
persistedModel = previousModel;
|
||||
throw error;
|
||||
}
|
||||
});
|
||||
|
||||
const res = await request(await createApp(transaction))
|
||||
.patch(`/api/issues/${existing.id}`)
|
||||
.send({ comment: "use Astra", assigneeAdapterOverrides: { adapterConfig: { model: "gpt-6-astra" } } });
|
||||
|
||||
expect(res.status).toBe(500);
|
||||
expect(transaction).toHaveBeenCalledOnce();
|
||||
expect(persistedModel).toBe("gpt-6-sol");
|
||||
expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("interrupts the active run and wakes the newly assigned agent with handoff context", async () => {
|
||||
const existing = makeIssue({
|
||||
assigneeAgentId: PREVIOUS_AGENT_ID,
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { asc, eq } from "drizzle-orm";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
||||
import { sql } from "drizzle-orm";
|
||||
import {
|
||||
activityLog,
|
||||
@@ -41,7 +41,9 @@ import {
|
||||
deriveIssueCommentRunLogAttribution,
|
||||
ISSUE_LIST_MAX_LIMIT,
|
||||
issueService,
|
||||
readIssueCommentRunLogText,
|
||||
} from "../services/issues.ts";
|
||||
import { getRunLogStore } from "../services/run-log-store.js";
|
||||
import {
|
||||
WORKSPACE_WORKTREE_REQUIRES_PROJECT_CODE,
|
||||
WORKSPACE_WORKTREE_REQUIRES_PROJECT_MESSAGE,
|
||||
@@ -149,6 +151,155 @@ describeEmbeddedPostgres("issueService run attachment artifacts", () => {
|
||||
}, 20_000);
|
||||
});
|
||||
|
||||
describe("readIssueCommentRunLogText", () => {
|
||||
it("cancels timed-out storage reads so later listings can recover", async () => {
|
||||
let active = 0;
|
||||
const cleanups: Array<() => void> = [];
|
||||
const read = vi.spyOn(getRunLogStore(), "read").mockImplementation((_handle, options) =>
|
||||
new Promise((_resolve, reject) => {
|
||||
active += 1;
|
||||
let settled = false;
|
||||
const abort = () => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
active -= 1;
|
||||
reject(new DOMException("Read aborted", "AbortError"));
|
||||
};
|
||||
cleanups.push(abort);
|
||||
options?.signal?.addEventListener("abort", abort, { once: true });
|
||||
}),
|
||||
);
|
||||
vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] });
|
||||
const run = { runId: "run", logStore: "local_file", logRef: "test/run.ndjson", logBytes: null };
|
||||
const firstBatch = Array.from({ length: 8 }, () => readIssueCommentRunLogText(run));
|
||||
try {
|
||||
await vi.advanceTimersByTimeAsync(3_000);
|
||||
expect(await Promise.all(firstBatch)).toEqual(Array(8).fill(""));
|
||||
expect(active).toBe(0);
|
||||
read.mockResolvedValueOnce({ content: "storage recovered" });
|
||||
await expect(readIssueCommentRunLogText(run)).resolves.toBe("storage recovered");
|
||||
expect(read).toHaveBeenCalledTimes(9);
|
||||
} finally {
|
||||
for (const cleanup of cleanups) cleanup();
|
||||
await Promise.allSettled(firstBatch);
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
vi.useRealTimers();
|
||||
read.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps readable attribution evidence for concurrent listings", async () => {
|
||||
let release!: () => void;
|
||||
const gate = new Promise<void>((resolve) => { release = resolve; });
|
||||
const read = vi.spyOn(getRunLogStore(), "read").mockImplementation(async () => {
|
||||
await gate;
|
||||
return { content: "comment id: legacy-comment" };
|
||||
});
|
||||
const run = { runId: "run", logStore: "local_file", logRef: "test/run.ndjson", logBytes: null };
|
||||
const listings = Array.from({ length: 2 }, () =>
|
||||
Promise.all(Array.from({ length: 8 }, () => readIssueCommentRunLogText(run))),
|
||||
);
|
||||
try {
|
||||
release();
|
||||
for (const listing of listings) {
|
||||
expect(await listing).toEqual(Array(8).fill("comment id: legacy-comment"));
|
||||
}
|
||||
expect(read).toHaveBeenCalledTimes(16);
|
||||
} finally {
|
||||
release();
|
||||
await Promise.allSettled(listings);
|
||||
read.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it.each([null, 128])("keeps partial attribution evidence when storage fails with logBytes=%s", async (logBytes) => {
|
||||
const read = vi.spyOn(getRunLogStore(), "read").mockRejectedValue(new Error("Storage gateway unavailable"));
|
||||
const run = { runId: "run", logStore: "local_file", logRef: "test/run.ndjson", logBytes };
|
||||
try {
|
||||
await expect(readIssueCommentRunLogText(run)).resolves.toBe("");
|
||||
read.mockResolvedValueOnce({ content: "earlier evidence", nextOffset: 16 });
|
||||
await expect(readIssueCommentRunLogText(run)).resolves.toBe("earlier evidence");
|
||||
} finally {
|
||||
read.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("bounds reads that ignore cancellation and stops late pagination after the deadline", async () => {
|
||||
let release!: (value: { content: string; nextOffset: number }) => void;
|
||||
const stalled = new Promise<{ content: string; nextOffset: number }>((resolve) => {
|
||||
release = resolve;
|
||||
});
|
||||
const read = vi.spyOn(getRunLogStore(), "read")
|
||||
.mockResolvedValueOnce({ content: "earlier evidence", nextOffset: 16 })
|
||||
.mockReturnValueOnce(stalled);
|
||||
vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] });
|
||||
let result: string | undefined;
|
||||
const pending = readIssueCommentRunLogText({
|
||||
runId: "run", logStore: "local_file", logRef: "test/run.ndjson", logBytes: null,
|
||||
}).then((value) => { result = value; });
|
||||
try {
|
||||
await vi.advanceTimersByTimeAsync(3_000);
|
||||
expect(result).toBe("earlier evidence");
|
||||
expect(read.mock.calls[1]?.[1]?.signal?.aborted).toBe(true);
|
||||
release({ content: "too late", nextOffset: 24 });
|
||||
await pending;
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
expect(read).toHaveBeenCalledTimes(2);
|
||||
expect(result).toBe("earlier evidence");
|
||||
} finally {
|
||||
release({ content: "", nextOffset: 0 });
|
||||
await pending.catch(() => {});
|
||||
vi.useRealTimers();
|
||||
read.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it.each([null, 128, 0])("reads existing attribution markers with logBytes=%s", async (logBytes) => {
|
||||
const commentId = randomUUID();
|
||||
const runId = randomUUID();
|
||||
const agentId = randomUUID();
|
||||
const read = vi.spyOn(getRunLogStore(), "read")
|
||||
.mockResolvedValueOnce({ content: "comment id: ", nextOffset: 12 })
|
||||
.mockResolvedValueOnce({ content: commentId + "\n" });
|
||||
try {
|
||||
const logContent = await readIssueCommentRunLogText({
|
||||
runId, logStore: "local_file", logRef: "test/run.ndjson", logBytes,
|
||||
});
|
||||
const derived = deriveIssueCommentRunLogAttribution(
|
||||
[{
|
||||
id: commentId,
|
||||
authorAgentId: null,
|
||||
authorUserId: "local-board",
|
||||
createdByRunId: null,
|
||||
createdAt: new Date("2020-01-01T00:00:01Z"),
|
||||
}],
|
||||
[{
|
||||
runId,
|
||||
agentId,
|
||||
createdAt: new Date("2020-01-01T00:00:00Z"),
|
||||
startedAt: new Date("2020-01-01T00:00:00Z"),
|
||||
finishedAt: new Date("2020-01-01T00:00:02Z"),
|
||||
logContent,
|
||||
}],
|
||||
);
|
||||
if (logBytes === 0) {
|
||||
expect(read).not.toHaveBeenCalled();
|
||||
expect(derived.size).toBe(0);
|
||||
} else {
|
||||
expect(read).toHaveBeenCalledTimes(2);
|
||||
expect(read.mock.calls[1]?.[1]?.offset).toBe(12);
|
||||
expect(derived.get(commentId)).toEqual({
|
||||
derivedAuthorAgentId: agentId,
|
||||
derivedCreatedByRunId: runId,
|
||||
derivedAuthorSource: "run_log_comment_post",
|
||||
});
|
||||
}
|
||||
} finally {
|
||||
read.mockRestore();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("deriveIssueCommentRunLogAttribution", () => {
|
||||
it("recovers agent attribution from run logs that printed the posted comment id", () => {
|
||||
const commentId = randomUUID();
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { eq, sql } from "drizzle-orm";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { agentWakeupRequests, agents, companies, createDb, heartbeatRuns, heartbeatRunEvents, issueComments, issueThreadInteractions, issues } from "@paperclipai/db";
|
||||
import { agentWakeupRequests, agents, companies, createDb, heartbeatRuns, heartbeatRunEvents, issueComments, issueThreadInteractions, issues, secretAccessEvents } from "@paperclipai/db";
|
||||
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
|
||||
import { acceptSteeredIdentity, captureRunIdentity, initializeRunIdentity, listRunIdentityContexts, rejectSteeredIdentity, reserveSteeredIdentity } from "../services/run-identity.js";
|
||||
|
||||
@@ -217,11 +217,11 @@ const support = await getEmbeddedPostgresTestSupport();
|
||||
}
|
||||
});
|
||||
|
||||
it("does not deadlock identity initialization against a task mutation that also updates the run", async () => {
|
||||
it.each(["update", "no key update"] as const)("serializes identity initialization behind a task's %s lock", async (lockMode) => {
|
||||
const input = await seed();
|
||||
let initialization!: ReturnType<typeof initializeRunIdentity>;
|
||||
await db.transaction(async (tx) => {
|
||||
await tx.select().from(issues).where(eq(issues.id, input.issueId)).for("update");
|
||||
await tx.select().from(issues).where(eq(issues.id, input.issueId)).for(lockMode);
|
||||
const [backend] = await tx.execute(sql`select pg_backend_pid() as pid`) as unknown as Array<{ pid: number }>;
|
||||
initialization = initializeRunIdentity(db, { ...input, messageIds: [], responsibleUserId: "A", cause: "instruction" });
|
||||
// Wait until initialization is blocked by this task mutation, rather than
|
||||
@@ -241,6 +241,47 @@ const support = await getEmbeddedPostgresTestSupport();
|
||||
await expect(initialization).resolves.toMatchObject({ responsibleUserId: "A" });
|
||||
});
|
||||
|
||||
it.each(["initialize", "capture"] as const)(
|
||||
"can %s identity while an audit append holds foreign-key locks",
|
||||
async (operation) => {
|
||||
const input = await seed();
|
||||
if (operation === "capture") {
|
||||
await initializeRunIdentity(db, { ...input, messageIds: [], responsibleUserId: "A", cause: "instruction" });
|
||||
}
|
||||
// A real append holds KEY SHARE on both parent rows until it commits.
|
||||
// Bound the other connection's wait so a conflicting lock fails this
|
||||
// regression instead of leaving both transactions waiting for each other.
|
||||
const identityDb = createDb(`${database.connectionString}?options=-c%20lock_timeout%3D1000`, {
|
||||
maxConnections: 1,
|
||||
});
|
||||
const [settings] = await identityDb.execute(sql`show lock_timeout`);
|
||||
expect(settings?.lock_timeout).toBe("1s");
|
||||
await db.transaction(async (audit) => {
|
||||
await audit.insert(secretAccessEvents).values({
|
||||
companyId: input.companyId,
|
||||
heartbeatRunId: input.runId,
|
||||
issueId: input.issueId,
|
||||
provider: "local_encrypted",
|
||||
actorType: "agent",
|
||||
actorId: input.agentId,
|
||||
consumerType: "agent",
|
||||
consumerId: input.agentId,
|
||||
outcome: "granted",
|
||||
});
|
||||
if (operation === "initialize") {
|
||||
await expect(initializeRunIdentity(identityDb, {
|
||||
...input, messageIds: [], responsibleUserId: "A", cause: "instruction",
|
||||
})).resolves.toMatchObject({ responsibleUserId: "A" });
|
||||
} else {
|
||||
await expect(captureRunIdentity(identityDb, input)).resolves.toMatchObject({
|
||||
run: { responsibleUserId: "A" },
|
||||
context: { responsibleUserId: "A" },
|
||||
});
|
||||
}
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it("does not turn a company-default fallback into personal consent on continuation", async () => {
|
||||
const input = await seed();
|
||||
await initializeRunIdentity(db, { ...input, messageIds: [], responsibleUserId: "A", cause: "company_default" });
|
||||
|
||||
@@ -24,6 +24,11 @@ import {
|
||||
workspaceRuntimeServices,
|
||||
} from "@paperclipai/db";
|
||||
import { eq } from "drizzle-orm";
|
||||
import {
|
||||
buildExecutionWorkspaceAdapterConfig,
|
||||
parseIssueExecutionWorkspaceSettings,
|
||||
parseProjectExecutionWorkspacePolicy,
|
||||
} from "../services/execution-workspace-policy.ts";
|
||||
import {
|
||||
buildWorkspaceRuntimeDesiredStatePatch,
|
||||
cleanupExecutionWorkspaceArtifacts,
|
||||
@@ -869,6 +874,52 @@ describe("realizeExecutionWorkspace", () => {
|
||||
expect(second.branchName).toBe(first.branchName);
|
||||
});
|
||||
|
||||
it("retains the project provision command when an issue overrides its base branch", async () => {
|
||||
const repoRoot = await createTempRepo();
|
||||
await fs.mkdir(path.join(repoRoot, "scripts"));
|
||||
await fs.writeFile(
|
||||
path.join(repoRoot, "scripts", "provision-worktree.sh"),
|
||||
"#!/usr/bin/env bash\necho 'Unexpected repository provision fallback' >&2\nexit 1\n",
|
||||
);
|
||||
await runGit(repoRoot, ["add", "scripts/provision-worktree.sh"]);
|
||||
await runGit(repoRoot, ["commit", "-m", "Add fallback provisioner"]);
|
||||
await runGit(repoRoot, ["branch", "release"]);
|
||||
const config = buildExecutionWorkspaceAdapterConfig({
|
||||
agentConfig: {},
|
||||
projectPolicy: parseProjectExecutionWorkspacePolicy({
|
||||
enabled: true,
|
||||
defaultMode: "isolated_workspace",
|
||||
workspaceStrategy: { type: "git_worktree", baseRef: "main", provisionCommand: "true" },
|
||||
}),
|
||||
issueSettings: parseIssueExecutionWorkspaceSettings({
|
||||
mode: "isolated_workspace",
|
||||
workspaceStrategy: { type: "git_worktree", baseRef: "release" },
|
||||
}),
|
||||
mode: "isolated_workspace",
|
||||
legacyUseProjectWorkspace: null,
|
||||
});
|
||||
try {
|
||||
const workspace = await realizeExecutionWorkspace({
|
||||
base: {
|
||||
baseCwd: repoRoot,
|
||||
source: "project_primary",
|
||||
projectId: "project-1",
|
||||
workspaceId: "workspace-1",
|
||||
repoUrl: null,
|
||||
repoRef: "HEAD",
|
||||
},
|
||||
config,
|
||||
issue: { id: "issue-1", identifier: "TEST-1", title: "Keep project setup" },
|
||||
agent: { id: "agent-1", name: "Test agent", companyId: "company-1" },
|
||||
});
|
||||
expect(workspace.created).toBe(true);
|
||||
expect(workspace.baseRefSha).toBe(await readGit(repoRoot, ["rev-parse", "release"]));
|
||||
await expect(fs.stat(path.join(workspace.cwd, ".git"))).resolves.toBeTruthy();
|
||||
} finally {
|
||||
await fs.rm(repoRoot, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("defaults the repo-provided worktree provisioner for git worktree strategies", async () => {
|
||||
const repoRoot = await createTempRepo();
|
||||
await fs.mkdir(path.join(repoRoot, "scripts"), { recursive: true });
|
||||
|
||||
@@ -1042,13 +1042,22 @@ function getDeclaredAdapterModels(): ReturnType<typeof parseAdapterModelsEnv> {
|
||||
return value;
|
||||
}
|
||||
|
||||
function declaredModelsForAdapter(type: string): { id: string; label: string }[] | null {
|
||||
const declared = getDeclaredAdapterModels()?.[type];
|
||||
return declared?.length
|
||||
? declared.map((model) => ({ id: model.id, label: model.label ?? model.id }))
|
||||
: null;
|
||||
}
|
||||
|
||||
export async function listAdapterModels(type: string): Promise<{ id: string; label: string }[]> {
|
||||
const declaredModels = getDeclaredAdapterModels();
|
||||
if (declaredModels && declaredModels[type]?.length) {
|
||||
return declaredModels[type].map((m) => ({ id: m.id, label: m.label ?? m.id }));
|
||||
}
|
||||
const declaredModels = declaredModelsForAdapter(type);
|
||||
if (declaredModels) return declaredModels;
|
||||
const adapter = findActiveServerAdapter(type);
|
||||
if (!adapter) return [];
|
||||
// The built-in Codex adapter's OpenAI discovery includes image, audio, and
|
||||
// embedding models that Codex cannot run. Use its curated list; declared
|
||||
// models above and custom adapter discovery remain authoritative.
|
||||
if (adapter === codexLocalAdapter) return adapter.models ?? [];
|
||||
if (adapter.listModels) {
|
||||
const discovered = await adapter.listModels();
|
||||
if (discovered.length > 0) return discovered;
|
||||
@@ -1057,6 +1066,9 @@ export async function listAdapterModels(type: string): Promise<{ id: string; lab
|
||||
}
|
||||
|
||||
export async function refreshAdapterModels(type: string): Promise<{ id: string; label: string }[]> {
|
||||
const declaredModels = declaredModelsForAdapter(type);
|
||||
if (declaredModels) return declaredModels;
|
||||
if (findActiveServerAdapter(type) === codexLocalAdapter) return listAdapterModels(type);
|
||||
const adapter = findActiveServerAdapter(type);
|
||||
if (!adapter) return [];
|
||||
if (adapter.refreshModels) {
|
||||
|
||||
@@ -50,7 +50,7 @@ Status quick guide:
|
||||
- Create subtasks with `POST /api/companies/{companyId}/issues`. Always set `parentId` and `goalId`. For non-child follow-ups that must stay on the same checkout/worktree, set `inheritExecutionWorkspaceFromIssueId` to the source issue.
|
||||
- When you know the needed work and owner, create those subtasks directly. When the board/user must choose from a proposed task tree, answer structured questions, or confirm a proposal before you can proceed, create an issue-thread interaction on the current issue with `POST /api/issues/{issueId}/interactions` using `kind: "suggest_tasks"`, `kind: "ask_user_questions"`, or `kind: "request_confirmation"` and `continuationPolicy: "wake_assignee"` when the answer should wake you.
|
||||
- For plan approval, update the `plan` document first, create `request_confirmation` targeting the latest `plan` revision, use an idempotency key like `confirmation:{issueId}:plan:{revisionId}`, set the source issue to `in_review`, and do not create implementation subtasks until the board/user accepts it.
|
||||
- `ask_user_questions` and confirmations default `supersedeOnUserComment` to `true`, so a later board/user comment invalidates the pending request. Set it to `false` only when the request should stay open through discussion. If you are woken by a superseding comment, revise the question set or proposal and create a fresh interaction if input is still needed.
|
||||
- `ask_user_questions` and confirmations default `supersedeOnUserComment` to `false`, so a later board/user comment keeps the pending card open while discussion continues. Set it to `true` when a new comment should replace the pending request. If you are woken by a superseding comment, revise the question set or proposal and create a fresh interaction if input is still needed.
|
||||
- Use `paperclip-create-agent` skill when hiring new agents.
|
||||
- Assign work to the right agent for the job.
|
||||
|
||||
|
||||
@@ -17,7 +17,7 @@ You are an agent at Paperclip company.
|
||||
3. Only then create `request_confirmation` with `target={ type: 'issue_document', key: 'plan', revisionId: latestRevisionId }` and `idempotencyKey=confirmation:{issueId}:plan:{revisionId}`.
|
||||
4. Wait for acceptance before creating implementation subtasks.
|
||||
Never present a plan only in a thread comment or through `ask_user_questions`; comments are supporting context and questions are for gathering input, not plan review.
|
||||
- `ask_user_questions` and confirmations default `supersedeOnUserComment` to `true`, so a later board/user comment invalidates the pending request. Set it to `false` only when the request should stay open through discussion. If you wake up from a superseding comment, revise the artifact, question set, or proposal and create a fresh interaction if input is still needed.
|
||||
- `ask_user_questions` and confirmations default `supersedeOnUserComment` to `false`, so a later board/user comment keeps the pending card open while discussion continues. Set it to `true` when a new comment should replace the pending request. If you wake up from a superseding comment, revise the artifact, question set, or proposal and create a fresh interaction if input is still needed.
|
||||
- For human input, save a pending question/confirmation interaction and set `in_review`; prose alone does not create a waiting path. Use `blockedByIssueIds` for issue dependencies. An agent may set an `unblockDescriptor` only for itself (`owner: { "agentId": "<your-agent-id>" }` plus `action`), not for the board/user or another agent.
|
||||
- Respect budget, pause/cancel, approval gates, and company boundaries.
|
||||
|
||||
|
||||
@@ -13729,13 +13729,17 @@ export function issueRoutes(
|
||||
});
|
||||
const decision =
|
||||
transition.decision && decisionId ? transition.decision : null;
|
||||
let attachmentComment: Awaited<ReturnType<typeof svc.addComment>> | null =
|
||||
let transactionalComment: Awaited<ReturnType<typeof svc.addComment>> | null =
|
||||
null;
|
||||
const attachmentCommentSourceTrust = commentAttachmentIds?.length
|
||||
const commentWithAdapterOverrides = Boolean(
|
||||
commentBody && updateFields.assigneeAdapterOverrides !== undefined,
|
||||
);
|
||||
const transactionalCommentSourceTrust = commentAttachmentIds?.length || commentWithAdapterOverrides
|
||||
? await sourceTrustForActorWrite(existing, actor)
|
||||
: undefined;
|
||||
const shouldUseTransactionalIssueUpdate =
|
||||
Boolean(commentAttachmentIds?.length) ||
|
||||
commentWithAdapterOverrides ||
|
||||
Boolean(decision) ||
|
||||
shouldRelayStop ||
|
||||
persistReviewActivityTransactionally ||
|
||||
@@ -13750,10 +13754,10 @@ export function issueRoutes(
|
||||
return null;
|
||||
const updated = await updateIssue(tx);
|
||||
if (!updated) return null;
|
||||
if (commentAttachmentIds?.length) {
|
||||
// Reassignment, comment creation and upload binding commit together.
|
||||
// An invalid or already-bound receipt rolls back the issue update.
|
||||
attachmentComment = await svc.addComment(
|
||||
if (commentAttachmentIds?.length || commentWithAdapterOverrides) {
|
||||
// Adapter settings, reassignment, comment and upload binding commit together.
|
||||
// A failed comment or invalid receipt rolls back the issue update.
|
||||
transactionalComment = await svc.addComment(
|
||||
id,
|
||||
commentBody,
|
||||
{
|
||||
@@ -13768,7 +13772,7 @@ export function issueRoutes(
|
||||
clientRequestId: actor.actorType === "user" ? commentClientRequestId : undefined,
|
||||
mirrorToSlack: actor.actorType === "user",
|
||||
authorizationReason: issueMutationAuthorizationReason,
|
||||
sourceTrust: attachmentCommentSourceTrust,
|
||||
sourceTrust: transactionalCommentSourceTrust,
|
||||
},
|
||||
tx,
|
||||
);
|
||||
@@ -14353,7 +14357,7 @@ export function issueRoutes(
|
||||
}
|
||||
|
||||
let comment: Awaited<ReturnType<typeof svc.addComment>> | null =
|
||||
attachmentComment;
|
||||
transactionalComment;
|
||||
let goalCommentSteered = false;
|
||||
let lostReviewPathRef: string | null = null;
|
||||
if (commentBody) {
|
||||
|
||||
@@ -12006,7 +12006,11 @@ export function chatChannelService(db: Db, options: ChatChannelServiceOptions) {
|
||||
run.nativeIssueId === issueId &&
|
||||
run.status === "failed" &&
|
||||
run.errorCode === "adapter_failed" &&
|
||||
run.error === "runner_state_identity_mismatch" &&
|
||||
// A diagnostic reason does not change this failure category. The exact
|
||||
// checkpoint, cleanup receipt, and no-provider-work proofs below still
|
||||
// decide whether the original request can be retried.
|
||||
(run.error === "runner_state_identity_mismatch" ||
|
||||
run.error?.startsWith("runner_state_identity_mismatch: ")) &&
|
||||
run.nativePhase === "observed" &&
|
||||
coordinator.phase === "observed" &&
|
||||
coordinator.attempt === 0 &&
|
||||
|
||||
@@ -38,17 +38,17 @@ function parseExecutionWorkspaceStrategy(raw: unknown): ExecutionWorkspaceStrate
|
||||
}
|
||||
return {
|
||||
type,
|
||||
...(typeof parsed.baseRef === "string" ? { baseRef: parsed.baseRef } : {}),
|
||||
...(typeof parsed.branchTemplate === "string" ? { branchTemplate: parsed.branchTemplate } : {}),
|
||||
...(typeof parsed.baseRef === "string" || parsed.baseRef === null ? { baseRef: parsed.baseRef } : {}),
|
||||
...(typeof parsed.branchTemplate === "string" || parsed.branchTemplate === null ? { branchTemplate: parsed.branchTemplate } : {}),
|
||||
...(typeof parsed.existingBranch === "string" && parsed.existingBranch.trim().length > 0
|
||||
? { existingBranch: parsed.existingBranch.trim() }
|
||||
: {}),
|
||||
...(typeof parsed.worktreeParentDir === "string" ? { worktreeParentDir: parsed.worktreeParentDir } : {}),
|
||||
...(typeof parsed.provisionCommand === "string" ? { provisionCommand: parsed.provisionCommand } : {}),
|
||||
...(typeof parsed.runtimeProvisionCommand === "string"
|
||||
...(typeof parsed.worktreeParentDir === "string" || parsed.worktreeParentDir === null ? { worktreeParentDir: parsed.worktreeParentDir } : {}),
|
||||
...(typeof parsed.provisionCommand === "string" || parsed.provisionCommand === null ? { provisionCommand: parsed.provisionCommand } : {}),
|
||||
...(typeof parsed.runtimeProvisionCommand === "string" || parsed.runtimeProvisionCommand === null
|
||||
? { runtimeProvisionCommand: parsed.runtimeProvisionCommand }
|
||||
: {}),
|
||||
...(typeof parsed.teardownCommand === "string" ? { teardownCommand: parsed.teardownCommand } : {}),
|
||||
...(typeof parsed.teardownCommand === "string" || parsed.teardownCommand === null ? { teardownCommand: parsed.teardownCommand } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
@@ -418,11 +418,18 @@ export function buildExecutionWorkspaceAdapterConfig(input: {
|
||||
|
||||
if (hasWorkspaceControl) {
|
||||
if (input.mode === "isolated_workspace") {
|
||||
const strategy =
|
||||
input.issueSettings?.workspaceStrategy ??
|
||||
input.projectPolicy?.workspaceStrategy ??
|
||||
const projectStrategy = projectHasPolicy ? input.projectPolicy?.workspaceStrategy : undefined;
|
||||
const issueStrategy = input.issueSettings?.workspaceStrategy;
|
||||
// An issue that changes its branch still needs the project's setup hooks.
|
||||
// Do not carry those defaults into a different execution strategy.
|
||||
const strategy = issueStrategy && projectStrategy?.type === issueStrategy.type
|
||||
? { ...projectStrategy, ...issueStrategy }
|
||||
: issueStrategy ?? projectStrategy ??
|
||||
parseExecutionWorkspaceStrategy(nextConfig.workspaceStrategy) ??
|
||||
({ type: "git_worktree" } satisfies ExecutionWorkspaceStrategy);
|
||||
if (issueStrategy?.existingBranch && issueStrategy.branchTemplate === undefined && strategy !== issueStrategy) {
|
||||
delete strategy.branchTemplate;
|
||||
}
|
||||
nextConfig.workspaceStrategy = strategy as unknown as Record<string, unknown>;
|
||||
} else {
|
||||
delete nextConfig.workspaceStrategy;
|
||||
|
||||
@@ -24933,8 +24933,8 @@ export function heartbeatService(
|
||||
: null;
|
||||
|
||||
let logSummary: {
|
||||
bytes: number;
|
||||
sha256?: string;
|
||||
bytes: number | null;
|
||||
sha256?: string | null;
|
||||
compressed: boolean;
|
||||
} | null = null;
|
||||
if (handle) {
|
||||
@@ -25652,8 +25652,8 @@ export function heartbeatService(
|
||||
logger.error({ err, runId }, "heartbeat execution failed");
|
||||
|
||||
let logSummary: {
|
||||
bytes: number;
|
||||
sha256?: string;
|
||||
bytes: number | null;
|
||||
sha256?: string | null;
|
||||
compressed: boolean;
|
||||
} | null = null;
|
||||
if (handle) {
|
||||
|
||||
@@ -851,7 +851,7 @@ function normalizeCreateInteractionInput(
|
||||
...input,
|
||||
payload: {
|
||||
...input.payload,
|
||||
supersedeOnUserComment: input.payload.supersedeOnUserComment ?? true,
|
||||
supersedeOnUserComment: input.payload.supersedeOnUserComment ?? false,
|
||||
},
|
||||
};
|
||||
case "request_confirmation":
|
||||
@@ -859,7 +859,7 @@ function normalizeCreateInteractionInput(
|
||||
...input,
|
||||
payload: {
|
||||
...input.payload,
|
||||
supersedeOnUserComment: input.payload.supersedeOnUserComment ?? true,
|
||||
supersedeOnUserComment: input.payload.supersedeOnUserComment ?? false,
|
||||
},
|
||||
};
|
||||
case "request_checkbox_confirmation":
|
||||
@@ -867,7 +867,7 @@ function normalizeCreateInteractionInput(
|
||||
...input,
|
||||
payload: {
|
||||
...input.payload,
|
||||
supersedeOnUserComment: input.payload.supersedeOnUserComment ?? true,
|
||||
supersedeOnUserComment: input.payload.supersedeOnUserComment ?? false,
|
||||
},
|
||||
};
|
||||
case "request_item_verdicts":
|
||||
@@ -875,7 +875,7 @@ function normalizeCreateInteractionInput(
|
||||
...input,
|
||||
payload: {
|
||||
...input.payload,
|
||||
supersedeOnUserComment: input.payload.supersedeOnUserComment ?? true,
|
||||
supersedeOnUserComment: input.payload.supersedeOnUserComment ?? false,
|
||||
},
|
||||
};
|
||||
default:
|
||||
|
||||
@@ -231,6 +231,7 @@ const ISSUE_COMMENT_RUN_LOG_DERIVATION_MAX_LOG_BYTES = 2_000_000;
|
||||
const ISSUE_COMMENT_RUN_LOG_DERIVATION_CHUNK_BYTES = 256_000;
|
||||
const ISSUE_COMMENT_RUN_LOG_DERIVATION_END_SLACK_MS = 60_000;
|
||||
const ISSUE_COMMENT_RUN_LOG_DERIVATION_MAX_PARALLEL_READS = 8;
|
||||
const ISSUE_COMMENT_RUN_LOG_DERIVATION_TIMEOUT_MS = 3_000;
|
||||
export const ISSUE_CREATE_IDEMPOTENCY_KEY_RETENTION_DAYS = 7;
|
||||
const ISSUE_CREATE_IDEMPOTENCY_KEY_RETENTION_MS =
|
||||
ISSUE_CREATE_IDEMPOTENCY_KEY_RETENTION_DAYS * 24 * 60 * 60 * 1000;
|
||||
@@ -6542,6 +6543,76 @@ async function countBlockedInboxIssues(
|
||||
}, 0);
|
||||
}
|
||||
|
||||
export async function readIssueCommentRunLogText(run: {
|
||||
runId?: string | null;
|
||||
logStore: string | null;
|
||||
logRef: string | null;
|
||||
logBytes: number | null;
|
||||
}) {
|
||||
if (run.logStore !== "local_file" || !run.logRef) return "";
|
||||
// A timed-out finalization leaves size unknown even when earlier entries
|
||||
// exist. Read those logs within the same byte budget as a known-size log.
|
||||
if (run.logBytes !== null && (!Number.isFinite(run.logBytes) || run.logBytes <= 0)) return "";
|
||||
|
||||
const logRef = run.logRef;
|
||||
const store = getRunLogStore();
|
||||
let offset = 0;
|
||||
let content = "";
|
||||
let nextOffset: number | undefined = 0;
|
||||
const controller = new AbortController();
|
||||
let readTimer: NodeJS.Timeout | undefined;
|
||||
|
||||
const readChunks = async () => {
|
||||
while (nextOffset !== undefined) {
|
||||
controller.signal.throwIfAborted();
|
||||
const remainingBytes =
|
||||
ISSUE_COMMENT_RUN_LOG_DERIVATION_MAX_LOG_BYTES -
|
||||
Buffer.byteLength(content, "utf8");
|
||||
if (remainingBytes <= 0) break;
|
||||
const chunk = await store.read(
|
||||
{ store: "local_file", logRef },
|
||||
{
|
||||
offset,
|
||||
limitBytes: Math.min(ISSUE_COMMENT_RUN_LOG_DERIVATION_CHUNK_BYTES, remainingBytes),
|
||||
signal: controller.signal,
|
||||
},
|
||||
);
|
||||
controller.signal.throwIfAborted();
|
||||
content += chunk.content;
|
||||
nextOffset = chunk.nextOffset;
|
||||
offset = chunk.nextOffset ?? 0;
|
||||
}
|
||||
};
|
||||
|
||||
try {
|
||||
await Promise.race([
|
||||
readChunks(),
|
||||
new Promise<never>((_resolve, reject) => {
|
||||
readTimer = setTimeout(() => {
|
||||
const reason = new DOMException("Attribution log read timed out", "TimeoutError");
|
||||
// Cancellation closes storage work where supported, but filesystem
|
||||
// I/O can delay stream destruction. Keep the response deadline too.
|
||||
reject(reason);
|
||||
controller.abort(reason);
|
||||
}, ISSUE_COMMENT_RUN_LOG_DERIVATION_TIMEOUT_MS);
|
||||
readTimer.unref?.();
|
||||
}),
|
||||
]);
|
||||
} catch (err) {
|
||||
// Attribution enriches already-authorized comments. Missing, failed, or
|
||||
// stalled storage must not prevent listing them; keep any evidence read.
|
||||
// Do not log raw provider errors, which can contain credentialed URLs.
|
||||
logger.warn(
|
||||
{ runId: run.runId ?? undefined, logRef, status: err instanceof HttpError ? err.status : undefined },
|
||||
"could not read heartbeat run log while deriving optional issue comment metadata",
|
||||
);
|
||||
} finally {
|
||||
clearTimeout(readTimer);
|
||||
}
|
||||
|
||||
return content;
|
||||
}
|
||||
|
||||
export function issueService(db: Db) {
|
||||
const instanceSettings = instanceSettingsService(db);
|
||||
const treeControlSvc = issueTreeControlService(db);
|
||||
@@ -6760,54 +6831,6 @@ export function issueService(db: Db) {
|
||||
};
|
||||
}
|
||||
|
||||
async function readRunLogText(run: {
|
||||
runId?: string | null;
|
||||
logStore: string | null;
|
||||
logRef: string | null;
|
||||
logBytes: number | null;
|
||||
}) {
|
||||
if (run.logStore !== "local_file" || !run.logRef) return "";
|
||||
const logBytes = Number(run.logBytes ?? 0);
|
||||
if (!Number.isFinite(logBytes) || logBytes <= 0) return "";
|
||||
|
||||
const store = getRunLogStore();
|
||||
let offset = 0;
|
||||
let content = "";
|
||||
let nextOffset: number | undefined = 0;
|
||||
|
||||
try {
|
||||
while (nextOffset !== undefined) {
|
||||
const remainingBytes =
|
||||
ISSUE_COMMENT_RUN_LOG_DERIVATION_MAX_LOG_BYTES -
|
||||
Buffer.byteLength(content, "utf8");
|
||||
if (remainingBytes <= 0) break;
|
||||
const chunk = await store.read(
|
||||
{ store: "local_file", logRef: run.logRef },
|
||||
{
|
||||
offset,
|
||||
limitBytes: Math.min(
|
||||
ISSUE_COMMENT_RUN_LOG_DERIVATION_CHUNK_BYTES,
|
||||
remainingBytes,
|
||||
),
|
||||
},
|
||||
);
|
||||
content += chunk.content;
|
||||
nextOffset = chunk.nextOffset;
|
||||
offset = chunk.nextOffset ?? 0;
|
||||
}
|
||||
} catch (err) {
|
||||
if (err instanceof HttpError && err.status === 404) {
|
||||
logger.warn(
|
||||
{ err, runId: run.runId ?? undefined, logRef: run.logRef },
|
||||
"missing heartbeat run log while deriving issue comment metadata",
|
||||
);
|
||||
return content;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
return content;
|
||||
}
|
||||
|
||||
// Persist a resolved attribution so subsequent reads stop re-scanning run
|
||||
// logs (and old "Board" threads stay fixed durably). Best-effort: a write
|
||||
@@ -7027,7 +7050,7 @@ export function issueService(db: Db) {
|
||||
);
|
||||
await Promise.all(
|
||||
batch.map(async (run) => {
|
||||
logByRunId.set(run.runId, await readRunLogText(run));
|
||||
logByRunId.set(run.runId, await readIssueCommentRunLogText(run));
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { readFileSync } from "node:fs";
|
||||
import {
|
||||
access,
|
||||
cp,
|
||||
@@ -57,6 +58,11 @@ import { buildNativeHeartbeatPreparationSpans } from "./native-run-trace.js";
|
||||
import { NativeRunnerOwnershipUnverifiedError } from "./native-runner-ownership.js";
|
||||
import type { AdapterRuntimeEvent } from "../../adapters/index.js";
|
||||
|
||||
vi.mock("node:fs", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("node:fs")>();
|
||||
return { ...actual, readFileSync: vi.fn(actual.readFileSync) };
|
||||
});
|
||||
|
||||
const githubAccess = vi.hoisted(() => ({
|
||||
activate: vi.fn((_binding: { runId: string }) => vi.fn()),
|
||||
stop: vi.fn(async () => undefined),
|
||||
@@ -1201,6 +1207,54 @@ describe("remote provider pack manifest", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("provider pack read diagnostics", () => {
|
||||
it.each([
|
||||
["EACCES", "permission_denied"],
|
||||
["EPERM", "permission_denied"],
|
||||
["EIO", "io_error"],
|
||||
])("reports %s without exposing the underlying filesystem message", (code, reason) => {
|
||||
const root = join(tmpdir(), "private-provider-pack");
|
||||
const manifestPath = join(root, "provider-pack.json");
|
||||
const cause = Object.assign(new Error(`${code}: cannot read ${manifestPath}`), {
|
||||
code,
|
||||
path: manifestPath,
|
||||
});
|
||||
vi.mocked(readFileSync).mockImplementationOnce(() => { throw cause; });
|
||||
let failure: Error | undefined;
|
||||
try { readRemoteProviderPackManifest(root); } catch (error) { failure = error as Error; }
|
||||
expect(readFileSync).toHaveBeenLastCalledWith(manifestPath, "utf8");
|
||||
expect(failure?.message).toBe(`runner_remote_provider_artifact_incompatible: provider-pack.json is unreadable (${reason})`);
|
||||
expect(failure?.message).not.toContain(root);
|
||||
expect(failure?.message).not.toContain(code);
|
||||
expect(failure?.cause).toBe(cause);
|
||||
});
|
||||
|
||||
it("classifies a JSON null manifest as incompatible instead of a TypeError", async () => {
|
||||
const root = await mkdtemp(join(tmpdir(), "paperclip-null-pack-"));
|
||||
try {
|
||||
await writeFile(join(root, "provider-pack.json"), "null");
|
||||
expect(() => readRemoteProviderPackManifest(root)).toThrow(
|
||||
"runner_remote_provider_artifact_incompatible: provider pack pins or source revision do not match",
|
||||
);
|
||||
} finally { await rm(root, { recursive: true, force: true }); }
|
||||
});
|
||||
|
||||
it.each(["missing", "invalid_json", "invalid_path_type"])("reports %s without putting the path in the terminal message", async (reason) => {
|
||||
const root = await mkdtemp(join(tmpdir(), "paperclip-private-pack-"));
|
||||
try {
|
||||
const manifestPath = join(root, "provider-pack.json");
|
||||
if (reason === "invalid_json") await writeFile(manifestPath, "{ private-invalid-json");
|
||||
if (reason === "invalid_path_type") await mkdir(manifestPath);
|
||||
let failure: Error | undefined;
|
||||
try { readRemoteProviderPackManifest(root); } catch (error) { failure = error as Error; }
|
||||
expect(failure?.message).toBe(`runner_remote_provider_artifact_incompatible: provider-pack.json is unreadable (${reason})`);
|
||||
expect(failure?.message).not.toContain(root);
|
||||
expect(failure?.message).not.toContain("private-invalid-json");
|
||||
expect(failure?.cause).toBeDefined();
|
||||
} finally { await rm(root, { recursive: true, force: true }); }
|
||||
});
|
||||
});
|
||||
|
||||
describe("native harness persistence profiles", () => {
|
||||
const profile = (provider: Record<string, unknown>, driverKind: string) =>
|
||||
resolveNativeHarnessPersistenceProfile({
|
||||
@@ -1507,7 +1561,7 @@ describe("verified native harness backups", () => {
|
||||
},
|
||||
sourceProviderLeaseId: "sandbox-1",
|
||||
}),
|
||||
).toThrow("runner_harness_state_mismatch");
|
||||
).toThrow("runner_harness_state_mismatch: backup_provider_identity_missing");
|
||||
} finally {
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
@@ -9166,7 +9220,7 @@ describe("runnerd provider runtime wiring", () => {
|
||||
execution: currentExecution,
|
||||
runnerInstanceId: "runner-after-running-prior-scope",
|
||||
}),
|
||||
).rejects.toThrow("runner_state_identity_mismatch");
|
||||
).rejects.toThrow("runner_state_identity_mismatch: prior_owner_active");
|
||||
await expect(access(scopedRoot)).resolves.toBeUndefined();
|
||||
await expect(access(join(stateBase, "quarantine"))).rejects.toThrow();
|
||||
expect(state.createBackend).not.toHaveBeenCalled();
|
||||
@@ -10738,7 +10792,7 @@ describe("runnerd provider runtime wiring", () => {
|
||||
}
|
||||
// Ambiguous ordinary recovery must still fail closed. Only the runtime's
|
||||
// explicitly admitted replacement may retire these prior-session backups.
|
||||
await expect(prepareReplacement()).rejects.toThrow("runner_harness_state_mismatch");
|
||||
await expect(prepareReplacement()).rejects.toThrow("runner_harness_state_mismatch: backup_without_reusable_lease");
|
||||
await expect(backend.openReplacementSession!({
|
||||
identity: { runId: execution.binding.runId }, workingDirectory: execution.workspace.cwd,
|
||||
} as never, {} as never)).resolves.toBe(replacement);
|
||||
|
||||
@@ -1705,7 +1705,7 @@ function migrateLegacyRunnerdStateRoot(input: {
|
||||
// A legacy path does not encode the full session scope. A mismatch may be
|
||||
// valid live state owned by another agent/workspace, so refusing the claim
|
||||
// is safe but moving that ambiguous directory is not.
|
||||
throw new Error("runner_state_identity_mismatch");
|
||||
throw new Error("runner_state_identity_mismatch: legacy_owner_unverified");
|
||||
}
|
||||
if (
|
||||
exactRun &&
|
||||
@@ -1718,7 +1718,7 @@ function migrateLegacyRunnerdStateRoot(input: {
|
||||
)
|
||||
) {
|
||||
quarantineRunnerdStateRoot(input.legacy, "identity_indeterminate");
|
||||
throw new Error("runner_state_identity_mismatch");
|
||||
throw new Error("runner_state_identity_mismatch: legacy_authority_indeterminate");
|
||||
}
|
||||
try {
|
||||
renameSync(input.legacy, input.scoped);
|
||||
@@ -1742,7 +1742,7 @@ function migrateLegacyRunnerdStateRoot(input: {
|
||||
durableIdentityMatchesSession(scopedIdentity, input.execution),
|
||||
);
|
||||
if (!exactScopedRun && !sameVerifiedPriorRun) {
|
||||
throw new Error("runner_state_identity_mismatch");
|
||||
throw new Error("runner_state_identity_mismatch: migration_destination_owner_changed");
|
||||
}
|
||||
}
|
||||
return input.scoped;
|
||||
@@ -4587,7 +4587,7 @@ async function migrateRunnerdStateRootForExecution(input: {
|
||||
await recoverQuiescentRunnerdState({ ...input, scoped });
|
||||
}
|
||||
if (input.restartRecovery?.kind === "reattach_remote_runner" && !existsSync(scoped)) {
|
||||
throw new Error("runner_state_identity_mismatch");
|
||||
throw new Error("runner_state_identity_mismatch: remote_reattach_root_missing");
|
||||
}
|
||||
if (existsSync(scoped)) {
|
||||
if (!isSafeNativeStateDirectory(scoped)) {
|
||||
@@ -4607,14 +4607,14 @@ async function migrateRunnerdStateRootForExecution(input: {
|
||||
input.restartRecovery?.kind !== "reattach_remote_runner") {
|
||||
quarantineRunnerdStateRoot(scoped, "identity_indeterminate");
|
||||
}
|
||||
throw new Error("runner_state_identity_mismatch");
|
||||
throw new Error("runner_state_identity_mismatch: durable_identity_unreadable");
|
||||
}
|
||||
if (!durableIdentityMatchesSession(identity, input.execution)) {
|
||||
if (input.restartRecovery?.kind !== "reattach_existing_runner" &&
|
||||
input.restartRecovery?.kind !== "reattach_remote_runner") {
|
||||
quarantineRunnerdStateRoot(scoped, "identity_mismatch");
|
||||
}
|
||||
throw new Error("runner_state_identity_mismatch");
|
||||
throw new Error("runner_state_identity_mismatch: session_scope_mismatch");
|
||||
}
|
||||
if (input.restartRecovery?.kind === "bootstrap_incomplete") {
|
||||
if (runnerdStateProvesIncompleteBootstrap(scoped)) {
|
||||
@@ -4624,7 +4624,7 @@ async function migrateRunnerdStateRootForExecution(input: {
|
||||
// Database evidence alone cannot distinguish a never-connected runner
|
||||
// from a partially-persisted provider bootstrap. Only the durable PRP
|
||||
// root can authorize a fresh bootstrap; anything else stays fail-closed.
|
||||
throw new Error("runner_state_identity_mismatch");
|
||||
throw new Error("runner_state_identity_mismatch: bootstrap_not_proven_incomplete");
|
||||
}
|
||||
if (input.restartRecovery?.kind === "reattach_remote_runner") {
|
||||
await verifyRemoteRunnerReattachment({
|
||||
@@ -4645,7 +4645,7 @@ async function migrateRunnerdStateRootForExecution(input: {
|
||||
if (input.restartRecovery?.kind !== "reattach_existing_runner") {
|
||||
quarantineRunnerdStateRoot(scoped, "identity_indeterminate");
|
||||
}
|
||||
throw new Error("runner_state_identity_mismatch");
|
||||
throw new Error("runner_state_identity_mismatch: authority_indeterminate");
|
||||
}
|
||||
} else {
|
||||
const verification = await verifyPriorRunnerdStateForSessionScope({
|
||||
@@ -4668,7 +4668,7 @@ async function migrateRunnerdStateRootForExecution(input: {
|
||||
: "identity_indeterminate",
|
||||
);
|
||||
}
|
||||
throw new Error("runner_state_identity_mismatch");
|
||||
throw new Error(`runner_state_identity_mismatch: prior_owner_${verification}`);
|
||||
}
|
||||
}
|
||||
return;
|
||||
@@ -4686,7 +4686,7 @@ async function migrateRunnerdStateRootForExecution(input: {
|
||||
// Unlike the full-scope target above, this legacy name can legitimately
|
||||
// belong to another scope. Leave it in place for its owner and fail the
|
||||
// attempted migration visibly.
|
||||
throw new Error("runner_state_identity_mismatch");
|
||||
throw new Error("runner_state_identity_mismatch: legacy_session_scope_mismatch");
|
||||
}
|
||||
let verifiedPriorRunId: string | undefined;
|
||||
if (!durableIdentityMatchesExecution(identity, input.execution)) {
|
||||
@@ -4709,7 +4709,7 @@ async function migrateRunnerdStateRootForExecution(input: {
|
||||
// untouched because the legacy name may still belong to them.
|
||||
quarantineRunnerdStateRoot(legacy, "identity_indeterminate");
|
||||
}
|
||||
throw new Error("runner_state_identity_mismatch");
|
||||
throw new Error(`runner_state_identity_mismatch: legacy_prior_owner_${verification}`);
|
||||
}
|
||||
verifiedPriorRunId = identity.runId;
|
||||
}
|
||||
@@ -4759,7 +4759,7 @@ async function recoverQuiescentRunnerdState(input: {
|
||||
) {
|
||||
// Do not roll back to an older valid checkpoint when a newer quarantined
|
||||
// root contains unconfirmed work, even if the newer root is unreadable.
|
||||
throw new Error("runner_state_identity_mismatch");
|
||||
throw new Error("runner_state_identity_mismatch: quarantine_candidates_ambiguous");
|
||||
}
|
||||
const verified: Array<{
|
||||
root: string;
|
||||
@@ -4946,7 +4946,7 @@ async function recoverQuiescentRunnerdState(input: {
|
||||
) {
|
||||
// Known provider history is not permission to start a replacement when
|
||||
// recovery cannot prove a unique, settled owner.
|
||||
throw new Error("runner_state_identity_mismatch");
|
||||
throw new Error("runner_state_identity_mismatch: quarantine_owner_unverified");
|
||||
}
|
||||
return;
|
||||
}
|
||||
@@ -4965,14 +4965,14 @@ async function recoverQuiescentRunnerdState(input: {
|
||||
"recovery_state_too_large",
|
||||
).toString("utf8") !== expected
|
||||
) {
|
||||
throw new Error("runner_state_identity_mismatch");
|
||||
throw new Error("runner_state_identity_mismatch: recovery_evidence_changed");
|
||||
}
|
||||
}
|
||||
if (
|
||||
!localProcessDefinitelyGone(candidate.processPid) ||
|
||||
!localProcessDefinitelyGone(candidate.processGroupId, true)
|
||||
) {
|
||||
throw new Error("runner_state_identity_mismatch");
|
||||
throw new Error("runner_state_identity_mismatch: recovery_process_not_gone");
|
||||
}
|
||||
if (candidate.root !== input.scoped) {
|
||||
if (existsSync(input.scoped)) {
|
||||
@@ -5687,12 +5687,12 @@ export function buildNativeHarnessBackupManifest(input: {
|
||||
completedAt?: string;
|
||||
}): NativeHarnessBackupManifest {
|
||||
if (!providerSessionIdentityIsPresent(input.providerSessionIdentity)) {
|
||||
throw new Error("runner_harness_state_mismatch");
|
||||
throw new Error("runner_harness_state_mismatch: backup_provider_identity_missing");
|
||||
}
|
||||
const profile = resolveNativeHarnessPersistenceProfile(input.execution);
|
||||
const directories = profile.directories.map((directory) => {
|
||||
const path = resolve(input.backupRoot, directory.name);
|
||||
if (!existsSync(path)) throw new Error("runner_harness_state_mismatch");
|
||||
if (!existsSync(path)) throw new Error("runner_harness_state_mismatch: backup_directory_missing");
|
||||
return { name: directory.name, ...digestBackupDirectory(path) };
|
||||
});
|
||||
return {
|
||||
@@ -9399,14 +9399,22 @@ export function readRemoteProviderPackManifest(
|
||||
readFileSync(resolve(packRoot, "provider-pack.json"), "utf8"),
|
||||
) as RemoteProviderPackManifest;
|
||||
} catch (error) {
|
||||
// The terminal run report keeps the outer message, not the cause chain.
|
||||
// Keep a bounded reason there; raw filesystem errors include private paths.
|
||||
const code = (error as NodeJS.ErrnoException | null)?.code;
|
||||
const reason = error instanceof SyntaxError ? "invalid_json"
|
||||
: code === "ENOENT" ? "missing"
|
||||
: code === "EACCES" || code === "EPERM" ? "permission_denied"
|
||||
: code === "EISDIR" || code === "ENOTDIR" ? "invalid_path_type"
|
||||
: "io_error";
|
||||
throw new Error(
|
||||
"runner_remote_provider_artifact_incompatible: provider-pack.json is unreadable",
|
||||
`runner_remote_provider_artifact_incompatible: provider-pack.json is unreadable (${reason})`,
|
||||
{ cause: error },
|
||||
);
|
||||
}
|
||||
const payload = manifest?.payload;
|
||||
if (
|
||||
manifest.schema !== REMOTE_PROVIDER_PACK_SCHEMA ||
|
||||
manifest?.schema !== REMOTE_PROVIDER_PACK_SCHEMA ||
|
||||
!payload ||
|
||||
canonicalJson(payload.pins) !== canonicalJson(REMOTE_PROVIDER_PACK_PINS) ||
|
||||
canonicalJson(payload.acpxProfileDigests) !==
|
||||
@@ -11601,7 +11609,7 @@ async function createRunnerdBackendWithinSessionClaim(
|
||||
async () => {
|
||||
for (const directory of persistenceProfile.directories) {
|
||||
const targetPath = remotePersistencePath(directory);
|
||||
if (!targetPath) throw new Error("runner_harness_state_mismatch");
|
||||
if (!targetPath) throw new Error("runner_harness_state_mismatch: restore_target_unavailable");
|
||||
await stageRemoteRunnerDirectory({
|
||||
target: remoteTarget,
|
||||
runner: remoteCommandRunner,
|
||||
@@ -11629,7 +11637,7 @@ async function createRunnerdBackendWithinSessionClaim(
|
||||
canonicalJson(restored.providerSessionIdentity) !==
|
||||
canonicalJson(backup.manifest.providerSessionIdentity)
|
||||
) {
|
||||
throw new Error("runner_harness_state_mismatch");
|
||||
throw new Error("runner_harness_state_mismatch: restored_provider_identity_changed");
|
||||
}
|
||||
// A deliberately non-reusable environment receives a fresh provider lease
|
||||
// for every turn. Stamp that new lease as soon as the verified host backup
|
||||
@@ -11644,7 +11652,7 @@ async function createRunnerdBackendWithinSessionClaim(
|
||||
for (const directory of persistenceProfile.directories) {
|
||||
if (directory.location !== "filesystem") continue;
|
||||
const targetPath = remotePersistencePath(directory);
|
||||
if (!targetPath) throw new Error("runner_harness_state_mismatch");
|
||||
if (!targetPath) throw new Error("runner_harness_state_mismatch: bootstrap_target_unavailable");
|
||||
const escapedTarget = targetPath.replaceAll("'", "'\\''");
|
||||
const created = await remoteCommandRunner.execute({
|
||||
command: "sh",
|
||||
@@ -11824,7 +11832,7 @@ async function createRunnerdBackendWithinSessionClaim(
|
||||
// A continuation that has a durable backup but no recorded reusable
|
||||
// lease was not provider-confirmed lost. Never silently create a new
|
||||
// provider session from that ambiguous state.
|
||||
throw new Error("runner_harness_state_mismatch");
|
||||
throw new Error("runner_harness_state_mismatch: backup_without_reusable_lease");
|
||||
}
|
||||
}
|
||||
} else if (remoteTarget && remoteCommandRunner) {
|
||||
@@ -12008,7 +12016,7 @@ async function createRunnerdBackendWithinSessionClaim(
|
||||
!verified.runnerState ||
|
||||
!verified.providerSessionIdentity
|
||||
) {
|
||||
throw new Error("runner_harness_state_mismatch");
|
||||
throw new Error("runner_harness_state_mismatch: checkpoint_identity_incomplete");
|
||||
}
|
||||
const providerSessionIdentity = verified.providerSessionIdentity;
|
||||
|
||||
@@ -12023,7 +12031,7 @@ async function createRunnerdBackendWithinSessionClaim(
|
||||
for (const directory of persistenceProfile.directories) {
|
||||
const sourcePath = remotePersistencePath(directory);
|
||||
if (!sourcePath)
|
||||
throw new Error("runner_harness_state_mismatch");
|
||||
throw new Error("runner_harness_state_mismatch: checkpoint_source_unavailable");
|
||||
const targetPath = resolve(pendingRoot, directory.name);
|
||||
await syncRemoteRunnerDirectoryOut({
|
||||
runner: remoteCommandRunner,
|
||||
@@ -12033,7 +12041,7 @@ async function createRunnerdBackendWithinSessionClaim(
|
||||
excludeEntries: directory.excludeEntries,
|
||||
});
|
||||
if (!existsSync(targetPath)) {
|
||||
throw new Error("runner_harness_state_mismatch");
|
||||
throw new Error("runner_harness_state_mismatch: checkpoint_directory_missing");
|
||||
}
|
||||
}
|
||||
const manifest = buildNativeHarnessBackupManifest({
|
||||
|
||||
@@ -1700,7 +1700,7 @@ export class PaperclipRunnerToolAuthority {
|
||||
acceptLabel: normalizedPayload.acceptLabel ?? "Confirm",
|
||||
rejectLabel: normalizedPayload.rejectLabel ?? "Request changes",
|
||||
rejectRequiresReason: normalizedPayload.rejectRequiresReason ?? false,
|
||||
supersedeOnUserComment: normalizedPayload.supersedeOnUserComment ?? true,
|
||||
supersedeOnUserComment: normalizedPayload.supersedeOnUserComment ?? false,
|
||||
} : {}),
|
||||
},
|
||||
} as never, { agentId: this.binding.agentId, userId: null, identityContextId });
|
||||
|
||||
@@ -57,7 +57,13 @@ export async function explicitOperatorRunIdentity(
|
||||
export type RunIdentityContext = typeof runIdentityContexts.$inferSelect;
|
||||
type Executor = Pick<Db, "select" | "insert" | "update">;
|
||||
|
||||
/** Match task mutation ordering: lock the task before the run, never the reverse. */
|
||||
/**
|
||||
* Lock the task before the run, matching task mutation ordering. Identity
|
||||
* operations do not change parent keys: NO KEY UPDATE still serializes writers
|
||||
* and steering, while allowing audit inserts to check their foreign keys.
|
||||
* FOR UPDATE can deadlock with an append that holds KEY SHARE on the run and
|
||||
* then checks the task while identity capture holds the task and waits on the run.
|
||||
*/
|
||||
async function lockIdentityTask(
|
||||
executor: Pick<Db, "select">,
|
||||
companyId: string,
|
||||
@@ -85,7 +91,7 @@ async function lockIdentityTask(
|
||||
.select({ id: issues.id })
|
||||
.from(issues)
|
||||
.where(and(eq(issues.id, issueId), eq(issues.companyId, companyId)))
|
||||
.for("update");
|
||||
.for("no key update");
|
||||
}
|
||||
|
||||
async function append(
|
||||
@@ -181,7 +187,7 @@ export async function initializeRunIdentity(
|
||||
eq(heartbeatRuns.companyId, input.companyId),
|
||||
),
|
||||
)
|
||||
.for("update");
|
||||
.for("no key update");
|
||||
if (!run) throw forbidden("Run identity does not belong to this company");
|
||||
if (run.activeIdentityContextId) {
|
||||
const [current] = await tx
|
||||
@@ -345,7 +351,7 @@ export async function reserveSteeredIdentity(
|
||||
eq(heartbeatRuns.companyId, input.companyId),
|
||||
),
|
||||
)
|
||||
.for("update");
|
||||
.for("no key update");
|
||||
// Processes started before the broker rollout keep their original environment.
|
||||
if (!run?.activeIdentityContextId) return null;
|
||||
const [pending] = await tx
|
||||
@@ -439,7 +445,7 @@ export async function captureRunIdentity(
|
||||
eq(heartbeatRuns.agentId, input.agentId),
|
||||
),
|
||||
)
|
||||
.for("update");
|
||||
.for("no key update");
|
||||
if (!run || run.status !== "running")
|
||||
throw forbidden(
|
||||
"Credential acquisition requires this agent's active run",
|
||||
@@ -512,7 +518,7 @@ export async function reconcileSteeredIdentity(
|
||||
eq(heartbeatRuns.companyId, context.companyId),
|
||||
),
|
||||
)
|
||||
.for("update");
|
||||
.for("no key update");
|
||||
if (!run) return;
|
||||
await acceptSteeredIdentity(tx, context);
|
||||
});
|
||||
|
||||
@@ -0,0 +1,72 @@
|
||||
import { createServer } from "node:http";
|
||||
import { promises as fs } from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { createDurableRunLogStore } from "./run-log-store.js";
|
||||
import { createS3StorageProvider } from "../storage/s3-provider.js";
|
||||
|
||||
afterEach(() => vi.unstubAllEnvs());
|
||||
|
||||
describe("run-log read cancellation", () => {
|
||||
it.each(["head", "get", "body"])("closes a stalled S3 %s connection and permits a subsequent read", async (stage) => {
|
||||
// Exercise the real SDK against an on-host server. No provider credentials
|
||||
// or external network are used by this cancellation regression.
|
||||
vi.stubEnv("AWS_ACCESS_KEY_ID", "test-access-key");
|
||||
vi.stubEnv("AWS_SECRET_ACCESS_KEY", "test-secret-key");
|
||||
vi.stubEnv("AWS_SESSION_TOKEN", "");
|
||||
const basePath = await fs.mkdtemp(path.join(os.tmpdir(), "run-log-abort-"));
|
||||
let recover = false;
|
||||
let closed = false;
|
||||
let started!: () => void;
|
||||
const stalled = new Promise<void>((resolve) => { started = resolve; });
|
||||
const server = createServer((request, response) => {
|
||||
const shouldStall = !recover && (stage === "head" ? request.method === "HEAD" : request.method === "GET");
|
||||
if (shouldStall) {
|
||||
response.on("close", () => { closed = true; });
|
||||
if (stage === "body") {
|
||||
response.writeHead(200, { "Content-Length": "4" });
|
||||
response.write("d");
|
||||
}
|
||||
started();
|
||||
return;
|
||||
}
|
||||
response.writeHead(200, { "Content-Length": "4" });
|
||||
response.end(request.method === "HEAD" ? undefined : "data");
|
||||
});
|
||||
await new Promise<void>((resolve) => server.listen(0, "127.0.0.1", resolve));
|
||||
const address = server.address();
|
||||
if (!address || typeof address === "string") throw new Error("Test server did not bind");
|
||||
const provider = createS3StorageProvider({
|
||||
bucket: "test-bucket", region: "us-east-1", forcePathStyle: true,
|
||||
endpoint: `http://127.0.0.1:${address.port}`,
|
||||
});
|
||||
const store = createDurableRunLogStore({ basePath, s3: { provider } });
|
||||
const handle = { store: "local_file" as const, logRef: "missing.ndjson" };
|
||||
const controller = new AbortController();
|
||||
const read = store.read(handle, { signal: controller.signal });
|
||||
void read.catch(() => {});
|
||||
try {
|
||||
await stalled;
|
||||
controller.abort();
|
||||
await expect(read).rejects.toMatchObject({ name: "AbortError" });
|
||||
await vi.waitFor(() => expect(closed).toBe(true));
|
||||
recover = true;
|
||||
expect(await store.read(handle)).toEqual({ content: "data", nextOffset: undefined });
|
||||
} finally {
|
||||
controller.abort();
|
||||
server.closeAllConnections();
|
||||
await new Promise<void>((resolve) => server.close(() => resolve()));
|
||||
await fs.rm(basePath, { recursive: true, force: true });
|
||||
}
|
||||
}, 10_000);
|
||||
|
||||
it("rejects an already-cancelled local read before opening a file", async () => {
|
||||
const store = createDurableRunLogStore({ basePath: os.tmpdir() });
|
||||
const controller = new AbortController();
|
||||
controller.abort();
|
||||
await expect(store.read({ store: "local_file", logRef: "unused.ndjson" }, {
|
||||
signal: controller.signal,
|
||||
})).rejects.toMatchObject({ name: "AbortError" });
|
||||
});
|
||||
});
|
||||
@@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { promises as fs } from "node:fs";
|
||||
import path from "node:path";
|
||||
import os from "node:os";
|
||||
import { createHash } from "node:crypto";
|
||||
import { Readable } from "node:stream";
|
||||
import { createDurableRunLogStore } from "./run-log-store.js";
|
||||
import type { StorageProvider } from "../storage/types.js";
|
||||
@@ -100,6 +101,102 @@ describe("createDurableRunLogStore", () => {
|
||||
expect(objects.get(key)!.toString("utf8")).toContain("line-B");
|
||||
});
|
||||
|
||||
it.each(["finishes", "fails"])("waits for an accepted file append that %s before finalizing", async (outcome) => {
|
||||
const { provider, objects } = createMemoryProvider();
|
||||
const store = createDurableRunLogStore({ basePath: baseDir, s3: { provider } });
|
||||
const handle = await store.begin(begin);
|
||||
let release!: () => void;
|
||||
const gate = new Promise<void>((resolve) => { release = resolve; });
|
||||
const appendFile = fs.appendFile.bind(fs);
|
||||
const spy = vi.spyOn(fs, "appendFile").mockImplementationOnce(async (...args) => {
|
||||
await gate;
|
||||
if (outcome === "fails") throw new Error("disk unavailable");
|
||||
await appendFile(...args);
|
||||
});
|
||||
const append = store.append(handle, { stream: "stderr", chunk: "late diagnostic", ts: "t1" });
|
||||
// Observe the deliberate rejection independently of finalization.
|
||||
void append.catch(() => {});
|
||||
let finalized = false;
|
||||
const finalize = store.finalize(handle).then((summary) => {
|
||||
finalized = true;
|
||||
return summary;
|
||||
});
|
||||
try {
|
||||
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||
expect(finalized).toBe(false);
|
||||
release();
|
||||
if (outcome === "fails") await expect(append).rejects.toThrow("disk unavailable");
|
||||
else await append;
|
||||
const summary = await finalize;
|
||||
const local = await fs.readFile(path.join(baseDir, handle.logRef));
|
||||
expect(summary.bytes).toBe(local.length);
|
||||
expect(summary.sha256).toBe(createHash("sha256").update(local).digest("hex"));
|
||||
expect(objects.get(handle.logRef)).toEqual(local);
|
||||
expect(local.toString()).toBe(outcome === "fails" ? "" : JSON.stringify({
|
||||
ts: "t1", stream: "stderr", chunk: "late diagnostic",
|
||||
}) + "\n");
|
||||
} finally {
|
||||
release();
|
||||
await append.catch(() => {});
|
||||
await finalize;
|
||||
spy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("ignores appends once finalization starts so the durable snapshot stays immutable", async () => {
|
||||
const { provider, objects } = createMemoryProvider();
|
||||
const store = createDurableRunLogStore({ basePath: baseDir, s3: { provider } });
|
||||
const handle = await store.begin(begin);
|
||||
await store.append(handle, { stream: "stdout", chunk: "accepted", ts: "t1" });
|
||||
const finalize = store.finalize(handle);
|
||||
expect(await store.append(handle, { stream: "stderr", chunk: "too late", ts: "t2" })).toBe(0);
|
||||
const summary = await finalize;
|
||||
expect(await store.append(handle, { stream: "stderr", chunk: "also too late", ts: "t3" })).toBe(0);
|
||||
const local = await fs.readFile(path.join(baseDir, handle.logRef));
|
||||
expect(local.toString()).not.toContain("too late");
|
||||
expect(summary.bytes).toBe(local.length);
|
||||
expect(summary.sha256).toBe(createHash("sha256").update(local).digest("hex"));
|
||||
expect(objects.get(handle.logRef)).toEqual(local);
|
||||
});
|
||||
|
||||
it("leaves final metadata unknown when an append stalls and never mirrors its late completion", async () => {
|
||||
const { provider, calls } = createMemoryProvider();
|
||||
const store = createDurableRunLogStore({ basePath: baseDir, s3: { provider, inflightMirrorMs: 10_000 } });
|
||||
const handle = await store.begin(begin);
|
||||
let release!: () => void;
|
||||
const gate = new Promise<void>((resolve) => { release = resolve; });
|
||||
const appendFile = fs.appendFile.bind(fs);
|
||||
const spy = vi.spyOn(fs, "appendFile").mockImplementationOnce(async (...args) => {
|
||||
await gate;
|
||||
await appendFile(...args);
|
||||
});
|
||||
const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] });
|
||||
const append = store.append(handle, { stream: "stderr", chunk: "stalled diagnostic", ts: "t1" });
|
||||
let summary: Awaited<ReturnType<typeof store.finalize>> | undefined;
|
||||
const finalize = store.finalize(handle).then((result) => { summary = result; });
|
||||
try {
|
||||
await vi.advanceTimersByTimeAsync(3_000);
|
||||
expect(summary).toEqual({ bytes: null, sha256: null, compressed: false });
|
||||
expect(warn).toHaveBeenCalled();
|
||||
expect(calls.put).toBe(0);
|
||||
release();
|
||||
await append;
|
||||
await vi.advanceTimersByTimeAsync(20_000);
|
||||
await store.flushInflightMirrors!();
|
||||
expect(calls.put).toBe(0);
|
||||
expect(await store.finalize(handle)).toEqual(summary);
|
||||
expect(await store.append(handle, { stream: "stderr", chunk: "too late", ts: "t2" })).toBe(0);
|
||||
} finally {
|
||||
release();
|
||||
await append;
|
||||
await finalize;
|
||||
vi.useRealTimers();
|
||||
spy.mockRestore();
|
||||
warn.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it("falls back to S3 when the local file is gone (the pod-roll case that caused 'Run log not found')", async () => {
|
||||
const { provider } = createMemoryProvider();
|
||||
const store = createDurableRunLogStore({ basePath: baseDir, s3: { provider, keyPrefix: "run-logs" } });
|
||||
@@ -163,25 +260,15 @@ describe("createDurableRunLogStore", () => {
|
||||
expect(caughtUp.nextOffset).toBeUndefined();
|
||||
});
|
||||
|
||||
it("falls back to S3 when the local file vanishes between stat() and open (TOCTOU race)", async () => {
|
||||
const { provider } = createMemoryProvider();
|
||||
const store = createDurableRunLogStore({ basePath: baseDir, s3: { provider, keyPrefix: "run-logs" } });
|
||||
it("reads local pages without waiting for a separate metadata request", async () => {
|
||||
const store = createDurableRunLogStore({ basePath: baseDir });
|
||||
const handle = await store.begin(begin);
|
||||
await store.append(handle, { stream: "stdout", chunk: "raced-line", ts: "t1" });
|
||||
await store.finalize(handle);
|
||||
// Delete the local file DURING stat(), i.e. after it reports the file
|
||||
// present but before createReadStream opens it -> the open hits ENOENT.
|
||||
const realStat = fs.stat.bind(fs);
|
||||
const statSpy = vi.spyOn(fs, "stat").mockImplementation(async (target, ...rest) => {
|
||||
const result = await realStat(target as Parameters<typeof realStat>[0], ...(rest as []));
|
||||
if (String(target).endsWith(".ndjson")) {
|
||||
await fs.rm(target as string, { force: true });
|
||||
}
|
||||
return result;
|
||||
});
|
||||
await fs.writeFile(path.join(baseDir, handle.logRef), "0123456789");
|
||||
const statSpy = vi.spyOn(fs, "stat").mockRejectedValue(new Error("Metadata unavailable"));
|
||||
try {
|
||||
const res = await store.read(handle);
|
||||
expect(res.content).toContain("raced-line");
|
||||
expect(await store.read(handle, { offset: 2, limitBytes: 4 })).toEqual({ content: "2345", nextOffset: 6 });
|
||||
expect(await store.read(handle, { offset: 6, limitBytes: 4 })).toEqual({ content: "6789", nextOffset: undefined });
|
||||
expect(await store.read(handle, { offset: 20, limitBytes: 4 })).toEqual({ content: "", nextOffset: undefined });
|
||||
} finally {
|
||||
statSpy.mockRestore();
|
||||
}
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { createReadStream, promises as fs } from "node:fs";
|
||||
import path from "node:path";
|
||||
import { createHash } from "node:crypto";
|
||||
import { addAbortSignal } from "node:stream";
|
||||
import { notFound } from "../errors.js";
|
||||
import { resolvePaperclipInstanceRoot } from "../home-paths.js";
|
||||
import { createS3StorageProvider } from "../storage/s3-provider.js";
|
||||
@@ -16,6 +17,7 @@ export interface RunLogHandle {
|
||||
export interface RunLogReadOptions {
|
||||
offset?: number;
|
||||
limitBytes?: number;
|
||||
signal?: AbortSignal;
|
||||
}
|
||||
|
||||
export interface RunLogReadResult {
|
||||
@@ -24,8 +26,10 @@ export interface RunLogReadResult {
|
||||
}
|
||||
|
||||
export interface RunLogFinalizeSummary {
|
||||
bytes: number;
|
||||
sha256?: string;
|
||||
// Null means a stalled write prevented a verified final snapshot. Callers
|
||||
// can still settle the run without recording a false byte count or hash.
|
||||
bytes: number | null;
|
||||
sha256?: string | null;
|
||||
compressed: boolean;
|
||||
}
|
||||
|
||||
@@ -100,6 +104,13 @@ export function createDurableRunLogStore(options: DurableRunLogStoreOptions): Ru
|
||||
const s3 = options.s3;
|
||||
const s3Prefix = normalizeKeyPrefix(s3?.keyPrefix);
|
||||
const inflightMirrorMs = s3?.inflightMirrorMs && s3.inflightMirrorMs > 0 ? s3.inflightMirrorMs : 0;
|
||||
// A run owns the write handle returned by begin(). Diagnostics can be
|
||||
// dispatched without awaiting the rest of onLog (DB progress/live events),
|
||||
// but finalize must include every file append already accepted on that handle.
|
||||
// Weak collections let completed handles disappear with their owning runs.
|
||||
const pendingAppends = new WeakMap<RunLogHandle, Set<Promise<void>>>();
|
||||
const closingHandles = new WeakSet<RunLogHandle>();
|
||||
const abandonedHandles = new WeakSet<RunLogHandle>();
|
||||
|
||||
function s3Key(logRef: string): string {
|
||||
return s3Prefix ? `${s3Prefix}/${logRef}` : logRef;
|
||||
@@ -159,6 +170,7 @@ export function createDurableRunLogStore(options: DurableRunLogStoreOptions): Ru
|
||||
}
|
||||
|
||||
function scheduleInflightMirror(logRef: string, entry: InflightMirrorEntry): void {
|
||||
if (inflightMirrors.get(logRef) !== entry) return;
|
||||
if (entry.timer || entry.upload) return;
|
||||
const delay = Math.max(0, inflightMirrorMs - (Date.now() - entry.lastMirrorAt));
|
||||
entry.timer = setTimeout(() => {
|
||||
@@ -205,33 +217,28 @@ export function createDurableRunLogStore(options: DurableRunLogStoreOptions): Ru
|
||||
filePath: string,
|
||||
offset: number,
|
||||
limitBytes: number,
|
||||
signal?: AbortSignal,
|
||||
): Promise<RunLogReadResult | null> {
|
||||
const stat = await fs.stat(filePath).catch(() => null);
|
||||
if (!stat) return null;
|
||||
const start = Math.max(0, Math.min(offset, stat.size));
|
||||
// No lower clamp to `start`: when the reader is fully caught up
|
||||
// (offset === size) that clamp made end === start and produced a
|
||||
// 1-byte-past-EOF range instead of an empty read.
|
||||
const end = Math.min(start + limitBytes - 1, stat.size - 1);
|
||||
if (start > end) return { content: "", nextOffset: start < stat.size ? start : undefined };
|
||||
|
||||
signal?.throwIfAborted();
|
||||
const start = Math.max(0, offset);
|
||||
// Read one extra byte to discover whether another page exists. A single
|
||||
// abortable stream avoids an uncancellable stat before opening the file.
|
||||
const chunks: Buffer[] = [];
|
||||
try {
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
const stream = createReadStream(filePath, { start, end });
|
||||
stream.on("data", (chunk) => chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)));
|
||||
stream.on("error", reject);
|
||||
stream.on("end", () => resolve());
|
||||
});
|
||||
const stream = createReadStream(filePath, { start, end: start + limitBytes, signal });
|
||||
for await (const chunk of stream) {
|
||||
chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk));
|
||||
}
|
||||
} catch (err) {
|
||||
// File deleted between stat() and open (pod-roll cleanup racing a read):
|
||||
// treat as missing so the caller falls through to the S3 mirror instead
|
||||
// of surfacing the very "Run log not found" this store exists to prevent.
|
||||
signal?.throwIfAborted();
|
||||
// A missing file, including deletion before open, falls back to S3.
|
||||
if ((err as NodeJS.ErrnoException | null)?.code === "ENOENT") return null;
|
||||
throw err;
|
||||
}
|
||||
const content = Buffer.concat(chunks).toString("utf8");
|
||||
const nextOffset = end + 1 < stat.size ? end + 1 : undefined;
|
||||
signal?.throwIfAborted();
|
||||
const bytes = Buffer.concat(chunks);
|
||||
const content = bytes.subarray(0, limitBytes).toString("utf8");
|
||||
const nextOffset = bytes.length > limitBytes ? start + limitBytes : undefined;
|
||||
return { content, nextOffset };
|
||||
}
|
||||
|
||||
@@ -239,10 +246,13 @@ export function createDurableRunLogStore(options: DurableRunLogStoreOptions): Ru
|
||||
logRef: string,
|
||||
offset: number,
|
||||
limitBytes: number,
|
||||
signal?: AbortSignal,
|
||||
): Promise<RunLogReadResult> {
|
||||
signal?.throwIfAborted();
|
||||
if (!s3) throw notFound("Run log not found");
|
||||
const key = s3Key(logRef);
|
||||
const head = await s3.provider.headObject({ objectKey: key });
|
||||
const head = await s3.provider.headObject({ objectKey: key, signal });
|
||||
signal?.throwIfAborted();
|
||||
if (!head.exists) throw notFound("Run log not found");
|
||||
const total = head.contentLength ?? 0;
|
||||
const start = Math.max(0, Math.min(offset, total));
|
||||
@@ -253,13 +263,15 @@ export function createDurableRunLogStore(options: DurableRunLogStoreOptions): Ru
|
||||
const end = Math.min(start + limitBytes - 1, total - 1);
|
||||
if (total === 0 || start > end) return { content: "", nextOffset: start < total ? start : undefined };
|
||||
|
||||
const result = await s3.provider.getObject({ objectKey: key, range: { start, end } });
|
||||
const result = await s3.provider.getObject({ objectKey: key, range: { start, end }, signal });
|
||||
// Destroy a body that stalls after headers arrive, including a body
|
||||
// returned just after the caller cancelled the request.
|
||||
if (signal) addAbortSignal(signal, result.stream);
|
||||
const chunks: Buffer[] = [];
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
result.stream.on("data", (chunk) => chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)));
|
||||
result.stream.on("error", reject);
|
||||
result.stream.on("end", () => resolve());
|
||||
});
|
||||
for await (const chunk of result.stream) {
|
||||
chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk));
|
||||
}
|
||||
signal?.throwIfAborted();
|
||||
const content = Buffer.concat(chunks).toString("utf8");
|
||||
const nextOffset = end + 1 < total ? end + 1 : undefined;
|
||||
return { content, nextOffset };
|
||||
@@ -289,7 +301,7 @@ export function createDurableRunLogStore(options: DurableRunLogStoreOptions): Ru
|
||||
},
|
||||
|
||||
async append(handle, event) {
|
||||
if (handle.store !== "local_file") return 0;
|
||||
if (handle.store !== "local_file" || closingHandles.has(handle)) return 0;
|
||||
const absPath = resolveWithin(basePath, handle.logRef);
|
||||
const line = JSON.stringify({
|
||||
ts: event.ts,
|
||||
@@ -301,13 +313,47 @@ export function createDurableRunLogStore(options: DurableRunLogStoreOptions): Ru
|
||||
...(typeof event.seq === "number" && Number.isFinite(event.seq) ? { seq: event.seq } : {}),
|
||||
});
|
||||
const persisted = `${line}\n`;
|
||||
await fs.appendFile(absPath, persisted, "utf8");
|
||||
noteInflightAppend(handle.logRef);
|
||||
let pending = pendingAppends.get(handle);
|
||||
if (!pending) {
|
||||
pending = new Set();
|
||||
pendingAppends.set(handle, pending);
|
||||
}
|
||||
const write = fs.appendFile(absPath, persisted, "utf8").then(() => {
|
||||
if (!closingHandles.has(handle)) noteInflightAppend(handle.logRef);
|
||||
});
|
||||
pending.add(write);
|
||||
try {
|
||||
await write;
|
||||
} finally {
|
||||
pending.delete(write);
|
||||
}
|
||||
return Buffer.byteLength(persisted, "utf8");
|
||||
},
|
||||
|
||||
async finalize(handle) {
|
||||
if (handle.store !== "local_file") return { bytes: 0, compressed: false };
|
||||
if (abandonedHandles.has(handle)) return { bytes: null, sha256: null, compressed: false };
|
||||
// Close admission before the first await. Drain file writes only, not
|
||||
// heartbeat's later DB/live-event persistence, then freeze one consistent
|
||||
// byte count, hash, and durable copy. Late diagnostics cannot mutate it.
|
||||
closingHandles.add(handle);
|
||||
let drainTimer: NodeJS.Timeout | undefined;
|
||||
const drained = await Promise.race([
|
||||
Promise.allSettled(pendingAppends.get(handle) ?? []).then(() => true),
|
||||
new Promise<boolean>((resolve) => {
|
||||
drainTimer = setTimeout(() => resolve(false), 3_000);
|
||||
drainTimer.unref?.();
|
||||
}),
|
||||
]).finally(() => clearTimeout(drainTimer));
|
||||
if (!drained) {
|
||||
// An in-flight fs append cannot be cancelled safely. Do not hash or
|
||||
// mirror a file it may still change, and never re-arm mirroring when
|
||||
// that write eventually finishes. Terminal run status can still settle.
|
||||
abandonedHandles.add(handle);
|
||||
void retireInflightMirror(handle.logRef).catch(() => undefined);
|
||||
console.warn("[run-log-store] Pending log writes did not settle within 3000ms; final log size and hash are unknown.");
|
||||
return { bytes: null, sha256: null, compressed: false };
|
||||
}
|
||||
await retireInflightMirror(handle.logRef);
|
||||
const absPath = resolveWithin(basePath, handle.logRef);
|
||||
const stat = await fs.stat(absPath).catch(() => null);
|
||||
@@ -344,14 +390,15 @@ export function createDurableRunLogStore(options: DurableRunLogStoreOptions): Ru
|
||||
},
|
||||
|
||||
async read(handle, opts) {
|
||||
opts?.signal?.throwIfAborted();
|
||||
if (handle.store !== "local_file") throw notFound("Run log not found");
|
||||
const absPath = resolveWithin(basePath, handle.logRef);
|
||||
const offset = opts?.offset ?? 0;
|
||||
const limitBytes = opts?.limitBytes ?? 256_000;
|
||||
const local = await readLocalRange(absPath, offset, limitBytes);
|
||||
const local = await readLocalRange(absPath, offset, limitBytes, opts?.signal);
|
||||
if (local) return local;
|
||||
// Local file gone (pod rolled) -> serve from the S3 mirror if configured.
|
||||
return readS3Range(handle.logRef, offset, limitBytes);
|
||||
return readS3Range(handle.logRef, offset, limitBytes, opts?.signal);
|
||||
},
|
||||
|
||||
async flushInflightMirrors() {
|
||||
|
||||
@@ -6,7 +6,7 @@ import {
|
||||
PutObjectCommand,
|
||||
} from "@aws-sdk/client-s3";
|
||||
import { putS3Multipart } from "./s3-multipart.js";
|
||||
import { Readable } from "node:stream";
|
||||
import { addAbortSignal, Readable } from "node:stream";
|
||||
import type { StorageProvider, GetObjectResult, HeadObjectResult } from "./types.js";
|
||||
import { notFound, unprocessable } from "../errors.js";
|
||||
|
||||
@@ -106,10 +106,13 @@ export function createS3StorageProvider(config: S3ProviderConfig): StorageProvid
|
||||
Key: key,
|
||||
Range: input.range ? `bytes=${input.range.start}-${input.range.end}` : undefined,
|
||||
}),
|
||||
{ abortSignal: input.signal },
|
||||
);
|
||||
|
||||
const stream = await toReadableStream(output.Body);
|
||||
if (input.signal) addAbortSignal(input.signal, stream);
|
||||
return {
|
||||
stream: await toReadableStream(output.Body),
|
||||
stream,
|
||||
contentType: output.ContentType,
|
||||
contentLength: output.ContentLength,
|
||||
etag: output.ETag,
|
||||
@@ -130,6 +133,7 @@ export function createS3StorageProvider(config: S3ProviderConfig): StorageProvid
|
||||
Bucket: bucket,
|
||||
Key: key,
|
||||
}),
|
||||
{ abortSignal: input.signal },
|
||||
);
|
||||
|
||||
return {
|
||||
|
||||
@@ -12,6 +12,8 @@ export interface PutObjectInput {
|
||||
|
||||
export interface GetObjectInput {
|
||||
objectKey: string;
|
||||
// S3 reads cancel pending requests and their response streams.
|
||||
signal?: AbortSignal;
|
||||
range?: {
|
||||
start: number;
|
||||
end: number;
|
||||
|
||||
Reference in new issue
Block a user