diff --git a/AGENTS.md b/AGENTS.md index 89cefbef4b..3555bfcdaf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -123,7 +123,9 @@ pnpm test:release-smoke Run the browser suites only when your change touches them or when you are explicitly verifying CI/release flows. -Run this full check before claiming done: +For normal issue work, run the smallest relevant verification first. Do not default to repo-wide typecheck/build/test on every heartbeat when a narrower check is enough to prove the change. + +Run this full check before claiming repo work done in a PR-ready hand-off, or when the change scope is broad enough that targeted checks are not sufficient: ```sh pnpm -r typecheck diff --git a/packages/adapter-utils/src/server-utils.test.ts b/packages/adapter-utils/src/server-utils.test.ts index 4faceaae07..068474456e 100644 --- a/packages/adapter-utils/src/server-utils.test.ts +++ b/packages/adapter-utils/src/server-utils.test.ts @@ -254,6 +254,7 @@ describe("renderPaperclipWakePrompt", () => { it("keeps the default local-agent prompt action-oriented", () => { expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain("Start actionable work in this heartbeat"); expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain("do not stop at a plan"); + expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain("Prefer the smallest verification that proves the change"); expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain("Use child issues"); expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain("instead of polling agents, sessions, or processes"); expect(DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE).toContain("Create child issues directly when you know what needs to be done"); @@ -326,6 +327,34 @@ describe("renderPaperclipWakePrompt", () => { expect(prompt).toContain("PAP-1723 Finish blocker (todo)"); }); + it("renders loose review request instructions for execution handoffs", () => { + const prompt = renderPaperclipWakePrompt({ + reason: "execution_review_requested", + issue: { + id: "issue-1", + identifier: "PAP-2011", + title: "Review request handoff", + status: "in_review", + }, + executionStage: { + wakeRole: "reviewer", + stageId: "stage-1", + stageType: "review", + currentParticipant: { type: "agent", agentId: "agent-1" }, + returnAssignee: { type: "agent", agentId: "agent-2" }, + reviewRequest: { + instructions: "Please focus on edge cases and leave a short risk summary.", + }, + allowedActions: ["approve", "request_changes"], + }, + fallbackFetchNeeded: false, + }); + + expect(prompt).toContain("Review request instructions:"); + expect(prompt).toContain("Please focus on edge cases and leave a short risk summary."); + expect(prompt).toContain("You are waking as the active reviewer for this issue."); + }); + it("includes continuation and child issue summaries in structured wake context", () => { const payload = { reason: "issue_children_completed", diff --git a/packages/adapter-utils/src/server-utils.ts b/packages/adapter-utils/src/server-utils.ts index 9e40d4ae39..a66ccdc08e 100644 --- a/packages/adapter-utils/src/server-utils.ts +++ b/packages/adapter-utils/src/server-utils.ts @@ -87,6 +87,7 @@ export const DEFAULT_PAPERCLIP_AGENT_PROMPT_TEMPLATE = [ "Execution contract:", "- Start actionable work in this heartbeat; do not stop at a plan unless the issue asks for planning.", "- Leave durable progress in comments, documents, or work products with a clear next action.", + "- Prefer the smallest verification that proves the change; do not default to full workspace typecheck/build/test on every heartbeat unless the task scope warrants it.", "- Use child issues for parallel or long delegated work instead of polling agents, sessions, or processes.", "- If woken by a human comment on a dependency-blocked issue, respond or triage the comment without treating the blocked deliverable work as unblocked.", "- Create child issues directly when you know what needs to be done; use issue-thread interactions when the board/user must choose suggested tasks, answer structured questions, or confirm a proposal.", @@ -282,6 +283,9 @@ type PaperclipWakeExecutionStage = { stageType: string | null; currentParticipant: PaperclipWakeExecutionPrincipal | null; returnAssignee: PaperclipWakeExecutionPrincipal | null; + reviewRequest: { + instructions: string; + } | null; lastDecisionOutcome: string | null; allowedActions: string[]; }; @@ -484,11 +488,14 @@ function normalizePaperclipWakeExecutionStage(value: unknown): PaperclipWakeExec : []; const currentParticipant = normalizePaperclipWakeExecutionPrincipal(stage.currentParticipant); const returnAssignee = normalizePaperclipWakeExecutionPrincipal(stage.returnAssignee); + const reviewRequestRaw = parseObject(stage.reviewRequest); + const reviewInstructions = asString(reviewRequestRaw.instructions, "").trim(); + const reviewRequest = reviewInstructions ? { instructions: reviewInstructions } : null; const stageId = asString(stage.stageId, "").trim() || null; const stageType = asString(stage.stageType, "").trim() || null; const lastDecisionOutcome = asString(stage.lastDecisionOutcome, "").trim() || null; - if (!wakeRole && !stageId && !stageType && !currentParticipant && !returnAssignee && !lastDecisionOutcome && allowedActions.length === 0) { + if (!wakeRole && !stageId && !stageType && !currentParticipant && !returnAssignee && !reviewRequest && !lastDecisionOutcome && allowedActions.length === 0) { return null; } @@ -498,6 +505,7 @@ function normalizePaperclipWakeExecutionStage(value: unknown): PaperclipWakeExec stageType, currentParticipant, returnAssignee, + reviewRequest, lastDecisionOutcome, allowedActions, }; @@ -664,6 +672,13 @@ export function renderPaperclipWakePrompt( if (executionStage.allowedActions.length > 0) { lines.push(`- allowed actions: ${executionStage.allowedActions.join(", ")}`); } + if (executionStage.reviewRequest) { + lines.push( + "", + "Review request instructions:", + executionStage.reviewRequest.instructions, + ); + } lines.push(""); if (executionStage.wakeRole === "reviewer" || executionStage.wakeRole === "approver") { lines.push( diff --git a/packages/adapter-utils/src/types.ts b/packages/adapter-utils/src/types.ts index ff00a0e989..89df75853d 100644 --- a/packages/adapter-utils/src/types.ts +++ b/packages/adapter-utils/src/types.ts @@ -64,12 +64,16 @@ export interface AdapterRuntimeServiceReport { healthStatus?: "unknown" | "healthy" | "unhealthy"; } +export type AdapterExecutionErrorFamily = "transient_upstream"; + export interface AdapterExecutionResult { exitCode: number | null; signal: string | null; timedOut: boolean; errorMessage?: string | null; errorCode?: string | null; + errorFamily?: AdapterExecutionErrorFamily | null; + retryNotBefore?: string | null; errorMeta?: Record; usage?: UsageSummary; /** @@ -311,6 +315,13 @@ export interface ServerAdapterModule { supportsLocalAgentJwt?: boolean; models?: AdapterModel[]; listModels?: () => Promise; + /** + * Optional explicit refresh hook for model discovery. + * Use this when the adapter caches discovered models and needs a bypass path + * so the UI can fetch newly released models without waiting for cache expiry + * or a Paperclip code update. + */ + refreshModels?: () => Promise; agentConfigurationDoc?: string; /** * Optional lifecycle hook when an agent is approved/hired (join-request or hire_agent approval). diff --git a/packages/adapters/claude-local/src/server/execute.ts b/packages/adapters/claude-local/src/server/execute.ts index 6fa870c85d..a79e722466 100644 --- a/packages/adapters/claude-local/src/server/execute.ts +++ b/packages/adapters/claude-local/src/server/execute.ts @@ -39,7 +39,9 @@ import { parseClaudeStreamJson, describeClaudeFailure, detectClaudeLoginRequired, + extractClaudeRetryNotBefore, isClaudeMaxTurnsResult, + isClaudeTransientUpstreamError, isClaudeUnknownSessionError, } from "./parse.js"; import { resolveClaudeDesiredSkillNames } from "./skills.js"; @@ -625,16 +627,48 @@ export async function execute(ctx: AdapterExecutionContext): Promise) : null; const clearSessionForMaxTurns = isClaudeMaxTurnsResult(parsed); + const parsedIsError = asBoolean(parsed.is_error, false); + const failed = (proc.exitCode ?? 0) !== 0 || parsedIsError; + const errorMessage = failed + ? describeClaudeFailure(parsed) ?? `Claude exited with code ${proc.exitCode ?? -1}` + : null; + const transientUpstream = + failed && + !loginMeta.requiresLogin && + isClaudeTransientUpstreamError({ + parsed, + stdout: proc.stdout, + stderr: proc.stderr, + errorMessage, + }); + const transientRetryNotBefore = transientUpstream + ? extractClaudeRetryNotBefore({ + parsed, + stdout: proc.stdout, + stderr: proc.stderr, + errorMessage, + }) + : null; + const resolvedErrorCode = loginMeta.requiresLogin + ? "claude_auth_required" + : transientUpstream + ? "claude_transient_upstream" + : null; + const mergedResultJson: Record = { + ...parsed, + ...(transientUpstream ? { errorFamily: "transient_upstream" } : {}), + ...(transientRetryNotBefore ? { retryNotBefore: transientRetryNotBefore.toISOString() } : {}), + ...(transientRetryNotBefore ? { transientRetryNotBefore: transientRetryNotBefore.toISOString() } : {}), + }; return { exitCode: proc.exitCode, signal: proc.signal, timedOut: false, - errorMessage: - (proc.exitCode ?? 0) === 0 - ? null - : describeClaudeFailure(parsed) ?? `Claude exited with code ${proc.exitCode ?? -1}`, - errorCode: loginMeta.requiresLogin ? "claude_auth_required" : null, + errorMessage, + errorCode: resolvedErrorCode, + errorFamily: transientUpstream ? "transient_upstream" : null, + retryNotBefore: transientRetryNotBefore ? transientRetryNotBefore.toISOString() : null, errorMeta, usage, sessionId: resolvedSessionId, @@ -690,7 +756,7 @@ export async function execute(ctx: AdapterExecutionContext): Promise { + it("classifies the 'out of extra usage' subscription window failure as transient", () => { + expect( + isClaudeTransientUpstreamError({ + errorMessage: "You're out of extra usage · resets 4pm (America/Chicago)", + }), + ).toBe(true); + expect( + isClaudeTransientUpstreamError({ + parsed: { + is_error: true, + result: "You're out of extra usage. Resets at 4pm (America/Chicago).", + }, + }), + ).toBe(true); + }); + + it("classifies Anthropic API rate_limit_error and overloaded_error as transient", () => { + expect( + isClaudeTransientUpstreamError({ + parsed: { + is_error: true, + errors: [{ type: "rate_limit_error", message: "Rate limit reached for requests." }], + }, + }), + ).toBe(true); + expect( + isClaudeTransientUpstreamError({ + parsed: { + is_error: true, + errors: [{ type: "overloaded_error", message: "Overloaded" }], + }, + }), + ).toBe(true); + expect( + isClaudeTransientUpstreamError({ + stderr: "HTTP 429: Too Many Requests", + }), + ).toBe(true); + expect( + isClaudeTransientUpstreamError({ + stderr: "Bedrock ThrottlingException: slow down", + }), + ).toBe(true); + }); + + it("classifies the subscription 5-hour / weekly limit wording", () => { + expect( + isClaudeTransientUpstreamError({ + errorMessage: "Claude usage limit reached — weekly limit reached. Try again in 2 days.", + }), + ).toBe(true); + expect( + isClaudeTransientUpstreamError({ + errorMessage: "5-hour limit reached.", + }), + ).toBe(true); + }); + + it("does not classify login/auth failures as transient", () => { + expect( + isClaudeTransientUpstreamError({ + stderr: "Please log in. Run `claude login` first.", + }), + ).toBe(false); + }); + + it("does not classify max-turns or unknown-session as transient", () => { + expect( + isClaudeTransientUpstreamError({ + parsed: { subtype: "error_max_turns", result: "Maximum turns reached." }, + }), + ).toBe(false); + expect( + isClaudeTransientUpstreamError({ + parsed: { + result: "No conversation found with session id abc-123", + errors: [{ message: "No conversation found with session id abc-123" }], + }, + }), + ).toBe(false); + }); + + it("does not classify deterministic validation errors as transient", () => { + expect( + isClaudeTransientUpstreamError({ + errorMessage: "Invalid request_error: Unknown parameter 'foo'.", + }), + ).toBe(false); + }); +}); + +describe("extractClaudeRetryNotBefore", () => { + it("parses the 'resets 4pm' hint in its explicit timezone", () => { + const now = new Date("2026-04-22T15:15:00.000Z"); + const extracted = extractClaudeRetryNotBefore( + { errorMessage: "You're out of extra usage · resets 4pm (America/Chicago)" }, + now, + ); + expect(extracted?.toISOString()).toBe("2026-04-22T21:00:00.000Z"); + }); + + it("rolls forward past midnight when the reset time has already passed today", () => { + const now = new Date("2026-04-22T23:30:00.000Z"); + const extracted = extractClaudeRetryNotBefore( + { errorMessage: "Usage limit reached. Resets at 3:15 AM (UTC)." }, + now, + ); + expect(extracted?.toISOString()).toBe("2026-04-23T03:15:00.000Z"); + }); + + it("returns null when no reset hint is present", () => { + expect( + extractClaudeRetryNotBefore({ errorMessage: "Overloaded. Try again later." }, new Date()), + ).toBeNull(); + }); +}); diff --git a/packages/adapters/claude-local/src/server/parse.ts b/packages/adapters/claude-local/src/server/parse.ts index c41e16cdfb..4591aaad02 100644 --- a/packages/adapters/claude-local/src/server/parse.ts +++ b/packages/adapters/claude-local/src/server/parse.ts @@ -1,9 +1,19 @@ import type { UsageSummary } from "@paperclipai/adapter-utils"; -import { asString, asNumber, parseObject, parseJson } from "@paperclipai/adapter-utils/server-utils"; +import { + asString, + asNumber, + parseObject, + parseJson, +} from "@paperclipai/adapter-utils/server-utils"; const CLAUDE_AUTH_REQUIRED_RE = /(?:not\s+logged\s+in|please\s+log\s+in|please\s+run\s+`?claude\s+login`?|login\s+required|requires\s+login|unauthorized|authentication\s+required)/i; const URL_RE = /(https?:\/\/[^\s'"`<>()[\]{};,!?]+[^\s'"`<>()[\]{};,!.?:]+)/gi; +const CLAUDE_TRANSIENT_UPSTREAM_RE = + /(?:rate[-\s]?limit(?:ed)?|rate_limit_error|too\s+many\s+requests|\b429\b|overloaded(?:_error)?|server\s+overloaded|service\s+unavailable|\b503\b|\b529\b|high\s+demand|try\s+again\s+later|temporarily\s+unavailable|throttl(?:ed|ing)|throttlingexception|servicequotaexceededexception|out\s+of\s+extra\s+usage|extra\s+usage\b|claude\s+usage\s+limit\s+reached|5[-\s]?hour\s+limit\s+reached|weekly\s+limit\s+reached|usage\s+limit\s+reached|usage\s+cap\s+reached)/i; +const CLAUDE_EXTRA_USAGE_RESET_RE = + /(?:out\s+of\s+extra\s+usage|extra\s+usage|usage\s+limit\s+reached|usage\s+cap\s+reached|5[-\s]?hour\s+limit\s+reached|weekly\s+limit\s+reached|claude\s+usage\s+limit\s+reached)[\s\S]{0,80}?\bresets?\s+(?:at\s+)?([^\n()]+?)(?:\s*\(([^)]+)\))?(?:[.!]|\n|$)/i; + export function parseClaudeStreamJson(stdout: string) { let sessionId: string | null = null; let model = ""; @@ -177,3 +187,197 @@ export function isClaudeUnknownSessionError(parsed: Record): bo /no conversation found with session id|unknown session|session .* not found/i.test(msg), ); } + +function buildClaudeTransientHaystack(input: { + parsed?: Record | null; + stdout?: string | null; + stderr?: string | null; + errorMessage?: string | null; +}): string { + const parsed = input.parsed ?? null; + const resultText = parsed ? asString(parsed.result, "") : ""; + const parsedErrors = parsed ? extractClaudeErrorMessages(parsed) : []; + return [ + input.errorMessage ?? "", + resultText, + ...parsedErrors, + input.stdout ?? "", + input.stderr ?? "", + ] + .join("\n") + .split(/\r?\n/) + .map((line) => line.trim()) + .filter(Boolean) + .join("\n"); +} + +function readTimeZoneParts(date: Date, timeZone: string) { + const values = new Map( + new Intl.DateTimeFormat("en-US", { + timeZone, + hourCycle: "h23", + year: "numeric", + month: "2-digit", + day: "2-digit", + hour: "2-digit", + minute: "2-digit", + }).formatToParts(date).map((part) => [part.type, part.value]), + ); + return { + year: Number.parseInt(values.get("year") ?? "", 10), + month: Number.parseInt(values.get("month") ?? "", 10), + day: Number.parseInt(values.get("day") ?? "", 10), + hour: Number.parseInt(values.get("hour") ?? "", 10), + minute: Number.parseInt(values.get("minute") ?? "", 10), + }; +} + +function normalizeResetTimeZone(timeZoneHint: string | null | undefined): string | null { + const normalized = timeZoneHint?.trim(); + if (!normalized) return null; + if (/^(?:utc|gmt)$/i.test(normalized)) return "UTC"; + + try { + new Intl.DateTimeFormat("en-US", { timeZone: normalized }).format(new Date(0)); + return normalized; + } catch { + return null; + } +} + +function dateFromTimeZoneWallClock(input: { + year: number; + month: number; + day: number; + hour: number; + minute: number; + timeZone: string; +}): Date | null { + let candidate = new Date(Date.UTC(input.year, input.month - 1, input.day, input.hour, input.minute, 0, 0)); + const targetUtc = Date.UTC(input.year, input.month - 1, input.day, input.hour, input.minute, 0, 0); + + for (let attempt = 0; attempt < 4; attempt += 1) { + const actual = readTimeZoneParts(candidate, input.timeZone); + const actualUtc = Date.UTC(actual.year, actual.month - 1, actual.day, actual.hour, actual.minute, 0, 0); + const offsetMs = targetUtc - actualUtc; + if (offsetMs === 0) break; + candidate = new Date(candidate.getTime() + offsetMs); + } + + const verified = readTimeZoneParts(candidate, input.timeZone); + if ( + verified.year !== input.year || + verified.month !== input.month || + verified.day !== input.day || + verified.hour !== input.hour || + verified.minute !== input.minute + ) { + return null; + } + + return candidate; +} + +function nextClockTimeInTimeZone(input: { + now: Date; + hour: number; + minute: number; + timeZoneHint: string; +}): Date | null { + const timeZone = normalizeResetTimeZone(input.timeZoneHint); + if (!timeZone) return null; + + const nowParts = readTimeZoneParts(input.now, timeZone); + let retryAt = dateFromTimeZoneWallClock({ + year: nowParts.year, + month: nowParts.month, + day: nowParts.day, + hour: input.hour, + minute: input.minute, + timeZone, + }); + if (!retryAt) return null; + + if (retryAt.getTime() <= input.now.getTime()) { + const nextDay = new Date(Date.UTC(nowParts.year, nowParts.month - 1, nowParts.day + 1, 0, 0, 0, 0)); + retryAt = dateFromTimeZoneWallClock({ + year: nextDay.getUTCFullYear(), + month: nextDay.getUTCMonth() + 1, + day: nextDay.getUTCDate(), + hour: input.hour, + minute: input.minute, + timeZone, + }); + } + + return retryAt; +} + +function parseClaudeResetClockTime(clockText: string, now: Date, timeZoneHint?: string | null): Date | null { + const normalized = clockText.trim().replace(/\s+/g, " "); + const match = normalized.match(/^(\d{1,2})(?::(\d{2}))?\s*([ap])\.?\s*m\.?/i); + if (!match) return null; + + const hour12 = Number.parseInt(match[1] ?? "", 10); + const minute = Number.parseInt(match[2] ?? "0", 10); + if (!Number.isInteger(hour12) || hour12 < 1 || hour12 > 12) return null; + if (!Number.isInteger(minute) || minute < 0 || minute > 59) return null; + + let hour24 = hour12 % 12; + if ((match[3] ?? "").toLowerCase() === "p") hour24 += 12; + + if (timeZoneHint) { + const explicitRetryAt = nextClockTimeInTimeZone({ + now, + hour: hour24, + minute, + timeZoneHint, + }); + if (explicitRetryAt) return explicitRetryAt; + } + + const retryAt = new Date(now); + retryAt.setHours(hour24, minute, 0, 0); + if (retryAt.getTime() <= now.getTime()) { + retryAt.setDate(retryAt.getDate() + 1); + } + return retryAt; +} + +export function extractClaudeRetryNotBefore( + input: { + parsed?: Record | null; + stdout?: string | null; + stderr?: string | null; + errorMessage?: string | null; + }, + now = new Date(), +): Date | null { + const haystack = buildClaudeTransientHaystack(input); + const match = haystack.match(CLAUDE_EXTRA_USAGE_RESET_RE); + if (!match) return null; + return parseClaudeResetClockTime(match[1] ?? "", now, match[2]); +} + +export function isClaudeTransientUpstreamError(input: { + parsed?: Record | null; + stdout?: string | null; + stderr?: string | null; + errorMessage?: string | null; +}): boolean { + const parsed = input.parsed ?? null; + // Deterministic failures are handled by their own classifiers. + if (parsed && (isClaudeMaxTurnsResult(parsed) || isClaudeUnknownSessionError(parsed))) { + return false; + } + const loginMeta = detectClaudeLoginRequired({ + parsed, + stdout: input.stdout ?? "", + stderr: input.stderr ?? "", + }); + if (loginMeta.requiresLogin) return false; + + const haystack = buildClaudeTransientHaystack(input); + if (!haystack) return false; + return CLAUDE_TRANSIENT_UPSTREAM_RE.test(haystack); +} diff --git a/packages/adapters/codex-local/src/server/execute.ts b/packages/adapters/codex-local/src/server/execute.ts index 87d14a7249..2a6b253a16 100644 --- a/packages/adapters/codex-local/src/server/execute.ts +++ b/packages/adapters/codex-local/src/server/execute.ts @@ -34,6 +34,7 @@ import { } from "@paperclipai/adapter-utils/server-utils"; import { parseCodexJsonl, + extractCodexRetryNotBefore, isCodexTransientUpstreamError, isCodexUnknownSessionError, } from "./parse.js"; @@ -725,6 +726,21 @@ export async function execute(ctx: AdapterExecutionContext): Promise { ).toBe(true); }); + it("classifies usage-limit windows as transient and extracts the retry time", () => { + const errorMessage = "You've hit your usage limit for GPT-5.3-Codex-Spark. Switch to another model now, or try again at 11:31 PM."; + const now = new Date(2026, 3, 22, 22, 29, 2); + + expect(isCodexTransientUpstreamError({ errorMessage })).toBe(true); + expect(extractCodexRetryNotBefore({ errorMessage }, now)?.getTime()).toBe( + new Date(2026, 3, 22, 23, 31, 0, 0).getTime(), + ); + }); + + it("parses explicit timezone hints on usage-limit retry windows", () => { + const errorMessage = "You've hit your usage limit for GPT-5.3-Codex-Spark. Switch to another model now, or try again at 11:31 PM (America/Chicago)."; + const now = new Date("2026-04-23T03:29:02.000Z"); + + expect(extractCodexRetryNotBefore({ errorMessage }, now)?.toISOString()).toBe( + "2026-04-23T04:31:00.000Z", + ); + }); + it("does not classify deterministic compaction errors as transient", () => { expect( isCodexTransientUpstreamError({ diff --git a/packages/adapters/codex-local/src/server/parse.ts b/packages/adapters/codex-local/src/server/parse.ts index c3ecff03ad..679a3f8f4d 100644 --- a/packages/adapters/codex-local/src/server/parse.ts +++ b/packages/adapters/codex-local/src/server/parse.ts @@ -1,8 +1,15 @@ -import { asString, asNumber, parseObject, parseJson } from "@paperclipai/adapter-utils/server-utils"; +import { + asString, + asNumber, + parseObject, + parseJson, +} from "@paperclipai/adapter-utils/server-utils"; const CODEX_TRANSIENT_UPSTREAM_RE = /(?:we(?:'|’)re\s+currently\s+experiencing\s+high\s+demand|temporary\s+errors|rate[-\s]?limit(?:ed)?|too\s+many\s+requests|\b429\b|server\s+overloaded|service\s+unavailable|try\s+again\s+later)/i; const CODEX_REMOTE_COMPACTION_RE = /remote\s+compact\s+task/i; +const CODEX_USAGE_LIMIT_RE = + /you(?:'|’)ve hit your usage limit for .+\.\s+switch to another model now,\s+or try again at\s+([^.!\n]+)(?:[.!]|\n|$)/i; export function parseCodexJsonl(stdout: string) { let sessionId: string | null = null; @@ -76,12 +83,12 @@ export function isCodexUnknownSessionError(stdout: string, stderr: string): bool ); } -export function isCodexTransientUpstreamError(input: { +function buildCodexErrorHaystack(input: { stdout?: string | null; stderr?: string | null; errorMessage?: string | null; -}): boolean { - const haystack = [ +}): string { + return [ input.errorMessage ?? "", input.stdout ?? "", input.stderr ?? "", @@ -91,9 +98,164 @@ export function isCodexTransientUpstreamError(input: { .map((line) => line.trim()) .filter(Boolean) .join("\n"); +} +function readTimeZoneParts(date: Date, timeZone: string) { + const values = new Map( + new Intl.DateTimeFormat("en-US", { + timeZone, + hourCycle: "h23", + year: "numeric", + month: "2-digit", + day: "2-digit", + hour: "2-digit", + minute: "2-digit", + }).formatToParts(date).map((part) => [part.type, part.value]), + ); + return { + year: Number.parseInt(values.get("year") ?? "", 10), + month: Number.parseInt(values.get("month") ?? "", 10), + day: Number.parseInt(values.get("day") ?? "", 10), + hour: Number.parseInt(values.get("hour") ?? "", 10), + minute: Number.parseInt(values.get("minute") ?? "", 10), + }; +} + +function normalizeResetTimeZone(timeZoneHint: string | null | undefined): string | null { + const normalized = timeZoneHint?.trim(); + if (!normalized) return null; + if (/^(?:utc|gmt)$/i.test(normalized)) return "UTC"; + + try { + new Intl.DateTimeFormat("en-US", { timeZone: normalized }).format(new Date(0)); + return normalized; + } catch { + return null; + } +} + +function dateFromTimeZoneWallClock(input: { + year: number; + month: number; + day: number; + hour: number; + minute: number; + timeZone: string; +}): Date | null { + let candidate = new Date(Date.UTC(input.year, input.month - 1, input.day, input.hour, input.minute, 0, 0)); + const targetUtc = Date.UTC(input.year, input.month - 1, input.day, input.hour, input.minute, 0, 0); + + for (let attempt = 0; attempt < 4; attempt += 1) { + const actual = readTimeZoneParts(candidate, input.timeZone); + const actualUtc = Date.UTC(actual.year, actual.month - 1, actual.day, actual.hour, actual.minute, 0, 0); + const offsetMs = targetUtc - actualUtc; + if (offsetMs === 0) break; + candidate = new Date(candidate.getTime() + offsetMs); + } + + const verified = readTimeZoneParts(candidate, input.timeZone); + if ( + verified.year !== input.year || + verified.month !== input.month || + verified.day !== input.day || + verified.hour !== input.hour || + verified.minute !== input.minute + ) { + return null; + } + + return candidate; +} + +function nextClockTimeInTimeZone(input: { + now: Date; + hour: number; + minute: number; + timeZoneHint: string; +}): Date | null { + const timeZone = normalizeResetTimeZone(input.timeZoneHint); + if (!timeZone) return null; + + const nowParts = readTimeZoneParts(input.now, timeZone); + let retryAt = dateFromTimeZoneWallClock({ + year: nowParts.year, + month: nowParts.month, + day: nowParts.day, + hour: input.hour, + minute: input.minute, + timeZone, + }); + if (!retryAt) return null; + + if (retryAt.getTime() <= input.now.getTime()) { + const nextDay = new Date(Date.UTC(nowParts.year, nowParts.month - 1, nowParts.day + 1, 0, 0, 0, 0)); + retryAt = dateFromTimeZoneWallClock({ + year: nextDay.getUTCFullYear(), + month: nextDay.getUTCMonth() + 1, + day: nextDay.getUTCDate(), + hour: input.hour, + minute: input.minute, + timeZone, + }); + } + + return retryAt; +} + +function parseLocalClockTime(clockText: string, now: Date): Date | null { + const normalized = clockText.trim(); + const match = normalized.match(/^(\d{1,2})(?::(\d{2}))?\s*([ap])\.?\s*m\.?(?:\s*\(([^)]+)\)|\s+([A-Z]{2,5}))?$/i); + if (!match) return null; + + const hour12 = Number.parseInt(match[1] ?? "", 10); + const minute = Number.parseInt(match[2] ?? "0", 10); + if (!Number.isInteger(hour12) || hour12 < 1 || hour12 > 12) return null; + if (!Number.isInteger(minute) || minute < 0 || minute > 59) return null; + + let hour24 = hour12 % 12; + if ((match[3] ?? "").toLowerCase() === "p") hour24 += 12; + + const timeZoneHint = match[4] ?? match[5]; + if (timeZoneHint) { + const explicitRetryAt = nextClockTimeInTimeZone({ + now, + hour: hour24, + minute, + timeZoneHint, + }); + if (explicitRetryAt) return explicitRetryAt; + } + + const retryAt = new Date(now); + retryAt.setHours(hour24, minute, 0, 0); + if (retryAt.getTime() <= now.getTime()) { + retryAt.setDate(retryAt.getDate() + 1); + } + return retryAt; +} + +export function extractCodexRetryNotBefore(input: { + stdout?: string | null; + stderr?: string | null; + errorMessage?: string | null; +}, now = new Date()): Date | null { + const haystack = buildCodexErrorHaystack(input); + const usageLimitMatch = haystack.match(CODEX_USAGE_LIMIT_RE); + if (!usageLimitMatch) return null; + return parseLocalClockTime(usageLimitMatch[1] ?? "", now); +} + +export function isCodexTransientUpstreamError(input: { + stdout?: string | null; + stderr?: string | null; + errorMessage?: string | null; +}): boolean { + const haystack = buildCodexErrorHaystack(input); + + if (extractCodexRetryNotBefore(input) != null) return true; if (!CODEX_TRANSIENT_UPSTREAM_RE.test(haystack)) return false; // Keep automatic retries scoped to the observed remote-compaction/high-demand - // failure shape; broader 429s may be caused by user or account limits. + // failure shape, plus explicit usage-limit windows that tell us when retrying + // becomes safe again. return CODEX_REMOTE_COMPACTION_RE.test(haystack) || /high\s+demand|temporary\s+errors/i.test(haystack); } diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index cad6d81cc9..b68d2d14af 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -631,6 +631,7 @@ export { updateIssueSchema, issueExecutionPolicySchema, issueExecutionStateSchema, + issueReviewRequestSchema, issueExecutionWorkspaceSettingsSchema, checkoutIssueSchema, addIssueCommentSchema, diff --git a/packages/shared/src/types/company-skill.ts b/packages/shared/src/types/company-skill.ts index 128340838e..29c3b58a51 100644 --- a/packages/shared/src/types/company-skill.ts +++ b/packages/shared/src/types/company-skill.ts @@ -59,6 +59,11 @@ export interface CompanySkillUsageAgent { urlKey: string; adapterType: string; desired: boolean; + /** + * Runtime adapter skill state when a caller explicitly fetched it. + * Company skill detail reads intentionally return null here to avoid probing + * agent runtimes while loading operator-facing skill metadata. + */ actualState: string | null; } diff --git a/packages/shared/src/types/index.ts b/packages/shared/src/types/index.ts index f23acff3ef..400721ceff 100644 --- a/packages/shared/src/types/index.ts +++ b/packages/shared/src/types/index.ts @@ -119,6 +119,7 @@ export type { IssueExecutionStage, IssueExecutionStageParticipant, IssueExecutionStagePrincipal, + IssueReviewRequest, IssueExecutionDecision, IssueComment, IssueThreadInteractionActorFields, diff --git a/packages/shared/src/types/issue.ts b/packages/shared/src/types/issue.ts index f105ab5c8e..b49094d063 100644 --- a/packages/shared/src/types/issue.ts +++ b/packages/shared/src/types/issue.ts @@ -168,6 +168,10 @@ export interface IssueExecutionPolicy { stages: IssueExecutionStage[]; } +export interface IssueReviewRequest { + instructions: string; +} + export interface IssueExecutionState { status: IssueExecutionStateStatus; currentStageId: string | null; @@ -175,6 +179,7 @@ export interface IssueExecutionState { currentStageType: IssueExecutionStageType | null; currentParticipant: IssueExecutionStagePrincipal | null; returnAssignee: IssueExecutionStagePrincipal | null; + reviewRequest: IssueReviewRequest | null; completedStageIds: string[]; lastDecisionId: string | null; lastDecisionOutcome: IssueExecutionDecisionOutcome | null; diff --git a/packages/shared/src/validators/company-skill.ts b/packages/shared/src/validators/company-skill.ts index 7f1df34b07..eb1df5ce57 100644 --- a/packages/shared/src/validators/company-skill.ts +++ b/packages/shared/src/validators/company-skill.ts @@ -43,7 +43,9 @@ export const companySkillUsageAgentSchema = z.object({ urlKey: z.string().min(1), adapterType: z.string().min(1), desired: z.boolean(), - actualState: z.string().nullable(), + actualState: z.string().nullable().describe( + "Runtime adapter skill state when explicitly fetched; company skill detail reads return null without probing agent runtimes.", + ), }); export const companySkillDetailSchema = companySkillSchema.extend({ diff --git a/packages/shared/src/validators/index.ts b/packages/shared/src/validators/index.ts index 5e8b22c578..5db0a16dc2 100644 --- a/packages/shared/src/validators/index.ts +++ b/packages/shared/src/validators/index.ts @@ -151,6 +151,7 @@ export { updateIssueSchema, issueExecutionPolicySchema, issueExecutionStateSchema, + issueReviewRequestSchema, issueExecutionWorkspaceSettingsSchema, checkoutIssueSchema, addIssueCommentSchema, diff --git a/packages/shared/src/validators/issue.ts b/packages/shared/src/validators/issue.ts index 6a0f2df9ae..0db2e1bcb9 100644 --- a/packages/shared/src/validators/issue.ts +++ b/packages/shared/src/validators/issue.ts @@ -105,6 +105,10 @@ export const issueExecutionPolicySchema = z.object({ stages: z.array(issueExecutionStageSchema).default([]), }); +export const issueReviewRequestSchema = z.object({ + instructions: z.string().trim().min(1).max(20000), +}).strict(); + export const issueExecutionStateSchema = z.object({ status: z.enum(ISSUE_EXECUTION_STATE_STATUSES), currentStageId: z.string().uuid().nullable(), @@ -112,6 +116,7 @@ export const issueExecutionStateSchema = z.object({ currentStageType: z.enum(ISSUE_EXECUTION_STAGE_TYPES).nullable(), currentParticipant: issueExecutionStagePrincipalSchema.nullable(), returnAssignee: issueExecutionStagePrincipalSchema.nullable(), + reviewRequest: issueReviewRequestSchema.nullable().optional().default(null), completedStageIds: z.array(z.string().uuid()).default([]), lastDecisionId: z.string().uuid().nullable(), lastDecisionOutcome: z.enum(ISSUE_EXECUTION_DECISION_OUTCOMES).nullable(), @@ -164,6 +169,7 @@ export type CreateIssueLabel = z.infer; export const updateIssueSchema = createIssueSchema.partial().extend({ assigneeAgentId: z.string().trim().min(1).optional().nullable(), comment: z.string().min(1).optional(), + reviewRequest: issueReviewRequestSchema.optional().nullable(), reopen: z.boolean().optional(), interrupt: z.boolean().optional(), hiddenAt: z.string().datetime().nullable().optional(), diff --git a/server/src/__tests__/adapter-model-refresh-routes.test.ts b/server/src/__tests__/adapter-model-refresh-routes.test.ts new file mode 100644 index 0000000000..5be7a5a0fe --- /dev/null +++ b/server/src/__tests__/adapter-model-refresh-routes.test.ts @@ -0,0 +1,185 @@ +import express from "express"; +import request from "supertest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { ServerAdapterModule } from "../adapters/index.js"; + +const mockAccessService = vi.hoisted(() => ({ + canUser: vi.fn(), + hasPermission: vi.fn(), + ensureMembership: vi.fn(), + setPrincipalPermission: vi.fn(), +})); + +const mockCompanySkillService = vi.hoisted(() => ({ + listRuntimeSkillEntries: vi.fn(), + resolveRequestedSkillKeys: vi.fn(), +})); + +const mockSecretService = vi.hoisted(() => ({ + normalizeAdapterConfigForPersistence: vi.fn(async (_companyId: string, config: Record) => config), + resolveAdapterConfigForRuntime: vi.fn(async (_companyId: string, config: Record) => ({ config })), +})); + +const mockAgentInstructionsService = vi.hoisted(() => ({ + materializeManagedBundle: vi.fn(), + getBundle: vi.fn(), + readFile: vi.fn(), + updateBundle: vi.fn(), + writeFile: vi.fn(), + deleteFile: vi.fn(), + exportFiles: vi.fn(), + ensureManagedBundle: vi.fn(), +})); + +const mockBudgetService = vi.hoisted(() => ({ + upsertPolicy: vi.fn(), +})); + +const mockHeartbeatService = vi.hoisted(() => ({ + cancelActiveForAgent: vi.fn(), +})); + +const mockIssueApprovalService = vi.hoisted(() => ({ + linkManyForApproval: vi.fn(), +})); + +const mockApprovalService = vi.hoisted(() => ({ + create: vi.fn(), + getById: vi.fn(), +})); + +const mockInstanceSettingsService = vi.hoisted(() => ({ + getGeneral: vi.fn(async () => ({ censorUsernameInLogs: false })), +})); + +const mockLogActivity = vi.hoisted(() => vi.fn()); + +function registerModuleMocks() { + vi.doMock("../services/index.js", () => ({ + agentService: () => ({}), + agentInstructionsService: () => mockAgentInstructionsService, + accessService: () => mockAccessService, + approvalService: () => mockApprovalService, + companySkillService: () => mockCompanySkillService, + budgetService: () => mockBudgetService, + heartbeatService: () => mockHeartbeatService, + issueApprovalService: () => mockIssueApprovalService, + issueService: () => ({}), + logActivity: mockLogActivity, + secretService: () => mockSecretService, + syncInstructionsBundleConfigFromFilePath: vi.fn((_agent, config) => config), + workspaceOperationService: () => ({}), + })); + + vi.doMock("../services/instance-settings.js", () => ({ + instanceSettingsService: () => mockInstanceSettingsService, + })); +} + +const refreshableAdapterType = "refreshable_adapter_route_test"; + +async function createApp() { + const [{ agentRoutes }, { errorHandler }] = await Promise.all([ + vi.importActual("../routes/agents.js"), + vi.importActual("../middleware/index.js"), + ]); + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + (req as any).actor = { + type: "board", + userId: "local-board", + companyIds: ["company-1"], + source: "local_implicit", + isInstanceAdmin: false, + }; + next(); + }); + app.use("/api", agentRoutes({} as any)); + app.use(errorHandler); + return app; +} + +async function requestApp( + app: express.Express, + buildRequest: (baseUrl: string) => request.Test, +) { + const { createServer } = await vi.importActual("node:http"); + const server = createServer(app); + try { + await new Promise((resolve) => { + server.listen(0, "127.0.0.1", resolve); + }); + const address = server.address(); + if (!address || typeof address === "string") { + throw new Error("Expected HTTP server to listen on a TCP port"); + } + return await buildRequest(`http://127.0.0.1:${address.port}`); + } finally { + if (server.listening) { + await new Promise((resolve, reject) => { + server.close((error) => { + if (error) reject(error); + else resolve(); + }); + }); + } + } +} + +async function unregisterTestAdapter(type: string) { + const { unregisterServerAdapter } = await import("../adapters/index.js"); + unregisterServerAdapter(type); +} + +describe("adapter model refresh route", () => { + beforeEach(async () => { + vi.resetModules(); + vi.doUnmock("../routes/agents.js"); + vi.doUnmock("../routes/authz.js"); + vi.doUnmock("../middleware/index.js"); + registerModuleMocks(); + vi.clearAllMocks(); + mockCompanySkillService.listRuntimeSkillEntries.mockResolvedValue([]); + mockCompanySkillService.resolveRequestedSkillKeys.mockResolvedValue([]); + mockAccessService.canUser.mockResolvedValue(true); + mockAccessService.hasPermission.mockResolvedValue(true); + mockAccessService.ensureMembership.mockResolvedValue(undefined); + mockAccessService.setPrincipalPermission.mockResolvedValue(undefined); + mockLogActivity.mockResolvedValue(undefined); + await unregisterTestAdapter(refreshableAdapterType); + }); + + afterEach(async () => { + await unregisterTestAdapter(refreshableAdapterType); + }); + + it("uses refreshModels when refresh=1 is requested", async () => { + const listModels = vi.fn(async () => [{ id: "stale-model", label: "stale-model" }]); + const refreshModels = vi.fn(async () => [{ id: "fresh-model", label: "fresh-model" }]); + const { registerServerAdapter } = await import("../adapters/index.js"); + const adapter: ServerAdapterModule = { + type: refreshableAdapterType, + execute: async () => ({ exitCode: 0, signal: null, timedOut: false }), + testEnvironment: async () => ({ + adapterType: refreshableAdapterType, + status: "pass", + checks: [], + testedAt: new Date(0).toISOString(), + }), + listModels, + refreshModels, + }; + registerServerAdapter(adapter); + + const app = await createApp(); + const res = await requestApp(app, (baseUrl) => + request(baseUrl).get(`/api/companies/company-1/adapters/${refreshableAdapterType}/models?refresh=1`), + ); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(res.body).toEqual([{ id: "fresh-model", label: "fresh-model" }]); + expect(refreshModels).toHaveBeenCalledTimes(1); + expect(listModels).not.toHaveBeenCalled(); + }); +}); diff --git a/server/src/__tests__/adapter-models.test.ts b/server/src/__tests__/adapter-models.test.ts index a6c5eb3584..2be936d98a 100644 --- a/server/src/__tests__/adapter-models.test.ts +++ b/server/src/__tests__/adapter-models.test.ts @@ -3,7 +3,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 } from "../adapters/index.js"; +import { listAdapterModels, refreshAdapterModels } from "../adapters/index.js"; import { resetCodexModelsCacheForTests } from "../adapters/codex-models.js"; import { resetCursorModelsCacheForTests, setCursorModelsRunnerForTests } from "../adapters/cursor-models.js"; @@ -52,6 +52,30 @@ describe("adapter model listing", () => { expect(first.some((model) => model.id === "codex-mini-latest")).toBe(true); }); + it("refreshes cached codex models on demand", 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.5" }], + }), + } as Response); + + 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.5")).toBe(true); + }); + it("falls back to static codex models when OpenAI model discovery fails", async () => { process.env.OPENAI_API_KEY = "sk-test"; vi.spyOn(globalThis, "fetch").mockResolvedValue({ diff --git a/server/src/__tests__/claude-local-execute.test.ts b/server/src/__tests__/claude-local-execute.test.ts index 386d8b1f24..96f5854b44 100644 --- a/server/src/__tests__/claude-local-execute.test.ts +++ b/server/src/__tests__/claude-local-execute.test.ts @@ -1,9 +1,23 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { execute } from "@paperclipai/adapter-claude-local/server"; +async function writeFailingClaudeCommand( + commandPath: string, + options: { resultEvent: Record; exitCode?: number }, +): Promise { + const payload = JSON.stringify(options.resultEvent); + const exit = options.exitCode ?? 1; + const script = `#!/usr/bin/env node +console.log(${JSON.stringify(payload)}); +process.exit(${exit}); +`; + await fs.writeFile(commandPath, script, "utf8"); + await fs.chmod(commandPath, 0o755); +} + async function writeFakeClaudeCommand(commandPath: string): Promise { const script = `#!/usr/bin/env node const fs = require("node:fs"); @@ -401,7 +415,7 @@ describe("claude execute", () => { const previousPaperclipInstanceId = process.env.PAPERCLIP_INSTANCE_ID; process.env.HOME = root; process.env.PAPERCLIP_HOME = paperclipHome; - delete process.env.PAPERCLIP_INSTANCE_ID; + process.env.PAPERCLIP_INSTANCE_ID = "default"; try { const first = await execute({ @@ -560,7 +574,7 @@ describe("claude execute", () => { const previousPaperclipInstanceId = process.env.PAPERCLIP_INSTANCE_ID; process.env.HOME = root; process.env.PAPERCLIP_HOME = paperclipHome; - delete process.env.PAPERCLIP_INSTANCE_ID; + process.env.PAPERCLIP_INSTANCE_ID = "default"; try { const first = await execute({ @@ -646,4 +660,179 @@ describe("claude execute", () => { await fs.rm(root, { recursive: true, force: true }); } }, 15_000); + + it("classifies Claude 'out of extra usage' failures as transient upstream errors", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-claude-execute-transient-")); + const workspace = path.join(root, "workspace"); + const commandPath = path.join(root, "claude"); + await fs.mkdir(workspace, { recursive: true }); + await writeFailingClaudeCommand(commandPath, { + resultEvent: { + type: "result", + subtype: "error", + session_id: "claude-session-extra", + is_error: true, + result: "You're out of extra usage · resets 4pm (America/Chicago)", + errors: [{ type: "rate_limit_error", message: "You're out of extra usage" }], + }, + }); + + const previousHome = process.env.HOME; + process.env.HOME = root; + vi.useFakeTimers(); + vi.setSystemTime(new Date(2026, 3, 22, 10, 15, 0)); + + try { + const result = await execute({ + runId: "run-claude-transient", + agent: { + id: "agent-1", + companyId: "company-1", + name: "Claude Coder", + adapterType: "claude_local", + adapterConfig: {}, + }, + runtime: { + sessionId: null, + sessionParams: null, + sessionDisplayId: null, + taskKey: null, + }, + config: { + command: commandPath, + cwd: workspace, + promptTemplate: "Follow the paperclip heartbeat.", + }, + context: {}, + authToken: "run-jwt-token", + onLog: async () => {}, + }); + + expect(result.exitCode).toBe(1); + expect(result.errorCode).toBe("claude_transient_upstream"); + expect(result.errorFamily).toBe("transient_upstream"); + expect(result.retryNotBefore).toBe("2026-04-22T21:00:00.000Z"); + expect(result.resultJson?.retryNotBefore).toBe("2026-04-22T21:00:00.000Z"); + expect(result.errorMessage ?? "").toContain("extra usage"); + expect(new Date(String(result.resultJson?.transientRetryNotBefore)).getTime()).toBe( + new Date("2026-04-22T21:00:00.000Z").getTime(), + ); + } finally { + vi.useRealTimers(); + if (previousHome === undefined) delete process.env.HOME; + else process.env.HOME = previousHome; + await fs.rm(root, { recursive: true, force: true }); + } + }); + + it("classifies rate-limit / overloaded failures without reset metadata as transient", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-claude-execute-rate-limit-")); + const workspace = path.join(root, "workspace"); + const commandPath = path.join(root, "claude"); + await fs.mkdir(workspace, { recursive: true }); + await writeFailingClaudeCommand(commandPath, { + resultEvent: { + type: "result", + subtype: "error", + session_id: "claude-session-overloaded", + is_error: true, + result: "Overloaded", + errors: [{ type: "overloaded_error", message: "Overloaded_error: API is overloaded." }], + }, + }); + + const previousHome = process.env.HOME; + process.env.HOME = root; + + try { + const result = await execute({ + runId: "run-claude-overloaded", + agent: { + id: "agent-1", + companyId: "company-1", + name: "Claude Coder", + adapterType: "claude_local", + adapterConfig: {}, + }, + runtime: { + sessionId: null, + sessionParams: null, + sessionDisplayId: null, + taskKey: null, + }, + config: { + command: commandPath, + cwd: workspace, + promptTemplate: "Follow the paperclip heartbeat.", + }, + context: {}, + authToken: "run-jwt-token", + onLog: async () => {}, + }); + + expect(result.exitCode).toBe(1); + expect(result.errorCode).toBe("claude_transient_upstream"); + expect(result.errorFamily).toBe("transient_upstream"); + expect(result.retryNotBefore ?? null).toBeNull(); + expect(result.resultJson?.retryNotBefore ?? null).toBeNull(); + expect(result.resultJson?.transientRetryNotBefore ?? null).toBeNull(); + } finally { + if (previousHome === undefined) delete process.env.HOME; + else process.env.HOME = previousHome; + await fs.rm(root, { recursive: true, force: true }); + } + }); + + it("does not reclassify deterministic Claude failures (auth, max turns) as transient", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-claude-execute-max-turns-")); + const workspace = path.join(root, "workspace"); + const commandPath = path.join(root, "claude"); + await fs.mkdir(workspace, { recursive: true }); + await writeFailingClaudeCommand(commandPath, { + resultEvent: { + type: "result", + subtype: "error_max_turns", + session_id: "claude-session-max-turns", + is_error: true, + result: "Maximum turns reached.", + }, + }); + + const previousHome = process.env.HOME; + process.env.HOME = root; + + try { + const result = await execute({ + runId: "run-claude-max-turns", + agent: { + id: "agent-1", + companyId: "company-1", + name: "Claude Coder", + adapterType: "claude_local", + adapterConfig: {}, + }, + runtime: { + sessionId: null, + sessionParams: null, + sessionDisplayId: null, + taskKey: null, + }, + config: { + command: commandPath, + cwd: workspace, + promptTemplate: "Follow the paperclip heartbeat.", + }, + context: {}, + authToken: "run-jwt-token", + onLog: async () => {}, + }); + + expect(result.exitCode).toBe(1); + expect(result.errorCode).not.toBe("claude_transient_upstream"); + } finally { + if (previousHome === undefined) delete process.env.HOME; + else process.env.HOME = previousHome; + await fs.rm(root, { recursive: true, force: true }); + } + }); }); diff --git a/server/src/__tests__/cleanup-removal-service.test.ts b/server/src/__tests__/cleanup-removal-service.test.ts index f74f3798f1..25dc11d679 100644 --- a/server/src/__tests__/cleanup-removal-service.test.ts +++ b/server/src/__tests__/cleanup-removal-service.test.ts @@ -7,8 +7,11 @@ import { companies, companySkills, createDb, + documents, + documentRevisions, heartbeatRuns, issueComments, + issueDocuments, issueExecutionDecisions, issueReadStates, issues, @@ -43,6 +46,8 @@ describeEmbeddedPostgres("cleanup removal services", () => { await db.delete(issueReadStates); await db.delete(issueComments); await db.delete(issueExecutionDecisions); + await db.delete(documentRevisions); + await db.delete(documents); await db.delete(companySkills); await db.delete(heartbeatRuns); await db.delete(issues); @@ -148,6 +153,8 @@ describeEmbeddedPostgres("cleanup removal services", () => { it("removes issue read states and activity rows before deleting the company", async () => { const { companyId, issueId, runId } = await seedFixture(); + const documentId = randomUUID(); + const revisionId = randomUUID(); await db.insert(issueReadStates).values({ id: randomUUID(), @@ -177,11 +184,47 @@ describeEmbeddedPostgres("cleanup removal services", () => { details: {}, }); + await db.insert(documents).values({ + id: documentId, + companyId, + title: "Run summary", + latestBody: "body", + latestRevisionId: revisionId, + latestRevisionNumber: 1, + createdByAgentId: null, + createdByUserId: "user-1", + updatedByAgentId: null, + updatedByUserId: "user-1", + }); + + await db.insert(issueDocuments).values({ + id: randomUUID(), + companyId, + issueId, + documentId, + key: "summary", + }); + + await db.insert(documentRevisions).values({ + id: revisionId, + companyId, + documentId, + revisionNumber: 1, + title: "Run summary", + format: "markdown", + body: "body", + createdByAgentId: null, + createdByUserId: "user-1", + createdByRunId: runId, + }); + const removed = await companyService(db).remove(companyId); expect(removed?.id).toBe(companyId); await expect(db.select().from(companies).where(eq(companies.id, companyId))).resolves.toHaveLength(0); await expect(db.select().from(issues).where(eq(issues.id, issueId))).resolves.toHaveLength(0); + await expect(db.select().from(documents).where(eq(documents.id, documentId))).resolves.toHaveLength(0); + await expect(db.select().from(documentRevisions).where(eq(documentRevisions.id, revisionId))).resolves.toHaveLength(0); await expect(db.select().from(issueReadStates).where(eq(issueReadStates.companyId, companyId))).resolves.toHaveLength(0); await expect(db.select().from(activityLog).where(eq(activityLog.companyId, companyId))).resolves.toHaveLength(0); }); diff --git a/server/src/__tests__/codex-local-execute.test.ts b/server/src/__tests__/codex-local-execute.test.ts index 54b790ad12..5a5bc9f493 100644 --- a/server/src/__tests__/codex-local-execute.test.ts +++ b/server/src/__tests__/codex-local-execute.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -419,6 +419,7 @@ describe("codex execute", () => { expect(result.exitCode).toBe(1); expect(result.errorCode).toBe("codex_transient_upstream"); + expect(result.errorFamily).toBe("transient_upstream"); expect(result.errorMessage).toContain("high demand"); } finally { if (previousHome === undefined) delete process.env.HOME; @@ -427,6 +428,68 @@ describe("codex execute", () => { } }); + it("persists retry-not-before metadata for codex usage-limit failures", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-execute-usage-limit-")); + const workspace = path.join(root, "workspace"); + const commandPath = path.join(root, "codex"); + await fs.mkdir(workspace, { recursive: true }); + await writeFailingCodexCommand( + commandPath, + "You've hit your usage limit for GPT-5.3-Codex-Spark. Switch to another model now, or try again at 11:31 PM.", + ); + + const previousHome = process.env.HOME; + process.env.HOME = root; + vi.useFakeTimers(); + vi.setSystemTime(new Date(2026, 3, 22, 22, 29, 0)); + + try { + const result = await execute({ + runId: "run-usage-limit", + agent: { + id: "agent-1", + companyId: "company-1", + name: "Codex Coder", + adapterType: "codex_local", + adapterConfig: {}, + }, + runtime: { + sessionId: "codex-session-usage-limit", + sessionParams: { + sessionId: "codex-session-usage-limit", + cwd: workspace, + }, + sessionDisplayId: "codex-session-usage-limit", + taskKey: null, + }, + config: { + command: commandPath, + cwd: workspace, + model: "gpt-5.3-codex-spark", + promptTemplate: "Follow the paperclip heartbeat.", + }, + context: {}, + authToken: "run-jwt-token", + onLog: async () => {}, + }); + + expect(result.exitCode).toBe(1); + expect(result.errorCode).toBe("codex_transient_upstream"); + expect(result.errorFamily).toBe("transient_upstream"); + const expectedRetryNotBefore = new Date(2026, 3, 22, 23, 31, 0, 0).toISOString(); + expect(result.retryNotBefore).toBe(expectedRetryNotBefore); + expect(result.resultJson?.retryNotBefore).toBe(expectedRetryNotBefore); + expect(new Date(String(result.resultJson?.transientRetryNotBefore)).getTime()).toBe( + new Date(2026, 3, 22, 23, 31, 0, 0).getTime(), + ); + } finally { + vi.useRealTimers(); + if (previousHome === undefined) delete process.env.HOME; + else process.env.HOME = previousHome; + await fs.rm(root, { recursive: true, force: true }); + } + }); + it("uses safer invocation settings and a fresh-session handoff for codex transient fallback retries", async () => { const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-codex-execute-fallback-")); const workspace = path.join(root, "workspace"); diff --git a/server/src/__tests__/company-skills-detail.test.ts b/server/src/__tests__/company-skills-detail.test.ts new file mode 100644 index 0000000000..726e95f231 --- /dev/null +++ b/server/src/__tests__/company-skills-detail.test.ts @@ -0,0 +1,232 @@ +import { randomUUID } from "node:crypto"; +import os from "node:os"; +import path from "node:path"; +import { promises as fs } from "node:fs"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { agents, companies, companySkills, createDb } from "@paperclipai/db"; +import { + getEmbeddedPostgresTestSupport, + startEmbeddedPostgresTestDatabase, +} from "./helpers/embedded-postgres.js"; +import { companySkillService } from "../services/company-skills.js"; + +const mockListSkills = vi.hoisted(() => vi.fn(() => new Promise(() => {}))); + +vi.mock("../adapters/index.js", async () => { + const actual = await vi.importActual("../adapters/index.js"); + return { + ...actual, + findActiveServerAdapter: vi.fn(() => ({ + listSkills: mockListSkills, + })), + }; +}); + +const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; + +if (!embeddedPostgresSupport.supported) { + console.warn( + `Skipping embedded Postgres company skill detail tests on this host: ${embeddedPostgresSupport.reason ?? "unsupported environment"}`, + ); +} + +describeEmbeddedPostgres("companySkillService.detail", () => { + let db!: ReturnType; + let svc!: ReturnType; + let tempDb: Awaited> | null = null; + const cleanupDirs = new Set(); + + beforeAll(async () => { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-company-skills-detail-"); + db = createDb(tempDb.connectionString); + svc = companySkillService(db); + }, 20_000); + + afterEach(async () => { + mockListSkills.mockClear(); + await db.delete(agents); + await db.delete(companySkills); + await db.delete(companies); + await Promise.all(Array.from(cleanupDirs, (dir) => fs.rm(dir, { recursive: true, force: true }))); + cleanupDirs.clear(); + }); + + afterAll(async () => { + await tempDb?.cleanup(); + }); + + function createTrackedDb(baseDb: ReturnType) { + const implicitCompanySkillSelects = vi.fn(); + + const trackedDb = new Proxy(baseDb, { + get(target, prop, receiver) { + if (prop !== "select") { + const value = Reflect.get(target, prop, receiver); + return typeof value === "function" ? value.bind(target) : value; + } + + return ((selection?: unknown) => { + const builder = selection === undefined ? target.select() : target.select(selection as never); + + return new Proxy(builder as object, { + get(builderTarget, builderProp, builderReceiver) { + if (builderProp !== "from") { + const value = Reflect.get(builderTarget, builderProp, builderReceiver); + return typeof value === "function" ? value.bind(builderTarget) : value; + } + + return (table: unknown) => { + const fromResult = (builderTarget as { from: (value: unknown) => unknown }).from(table); + if (table === companySkills) { + if (selection === undefined) { + implicitCompanySkillSelects(); + } + } + + return fromResult; + }; + }, + }); + }) as typeof target.select; + }, + }); + + return { + db: trackedDb as typeof baseDb, + implicitCompanySkillSelects, + }; + } + + it("reports attached agents without probing adapter runtime skill state", async () => { + const companyId = randomUUID(); + const skillId = randomUUID(); + const skillKey = `company/${companyId}/reflection-coach`; + const skillDir = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-reflection-skill-")); + cleanupDirs.add(skillDir); + await fs.writeFile(path.join(skillDir, "SKILL.md"), "# Reflection Coach\n", "utf8"); + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + await db.insert(companySkills).values({ + id: skillId, + companyId, + key: skillKey, + slug: "reflection-coach", + name: "Reflection Coach", + description: null, + markdown: "# Reflection Coach\n", + sourceType: "local_path", + sourceLocator: skillDir, + trustLevel: "markdown_only", + compatibility: "compatible", + fileInventory: [{ path: "SKILL.md", kind: "skill" }], + metadata: { sourceKind: "local_path" }, + }); + await db.insert(agents).values({ + id: randomUUID(), + companyId, + name: "Reviewer", + role: "engineer", + adapterType: "codex_local", + adapterConfig: { + paperclipSkillSync: { + desiredSkills: [skillKey], + }, + }, + }); + + const detail = await Promise.race([ + svc.detail(companyId, skillId), + new Promise((_, reject) => setTimeout(() => reject(new Error("skill detail timed out")), 1_000)), + ]); + + expect(mockListSkills).not.toHaveBeenCalled(); + expect(detail?.usedByAgents).toEqual([ + expect.objectContaining({ + name: "Reviewer", + desired: true, + actualState: null, + }), + ]); + }); + + it("uses explicit company skill column selections when resolving detail usage", async () => { + const companyId = randomUUID(); + const skillId = randomUUID(); + const skillKey = `company/${companyId}/reflection-coach`; + const skillDir = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-reflection-skill-")); + cleanupDirs.add(skillDir); + await fs.writeFile(path.join(skillDir, "SKILL.md"), "# Reflection Coach\n", "utf8"); + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + await db.insert(companySkills).values([ + { + id: skillId, + companyId, + key: skillKey, + slug: "reflection-coach", + name: "Reflection Coach", + description: null, + markdown: "# Reflection Coach\n", + sourceType: "local_path", + sourceLocator: skillDir, + trustLevel: "markdown_only", + compatibility: "compatible", + fileInventory: [{ path: "SKILL.md", kind: "skill" }], + metadata: { sourceKind: "local_path" }, + }, + { + id: randomUUID(), + companyId, + key: `company/${companyId}/large-reference-skill`, + slug: "large-reference-skill", + name: "Large Reference Skill", + description: null, + markdown: `# Large Reference Skill\n\n${"x".repeat(32_000)}`, + sourceType: "catalog", + sourceLocator: "paperclip://catalog/large-reference-skill", + trustLevel: "markdown_only", + compatibility: "compatible", + fileInventory: [{ path: "SKILL.md", kind: "skill" }], + metadata: { sourceKind: "catalog" }, + }, + ]); + await db.insert(agents).values({ + id: randomUUID(), + companyId, + name: "Reviewer", + role: "engineer", + adapterType: "codex_local", + adapterConfig: { + paperclipSkillSync: { + desiredSkills: ["reflection-coach"], + }, + }, + }); + + const tracked = createTrackedDb(db); + const trackedSvc = companySkillService(tracked.db); + const detail = await Promise.race([ + trackedSvc.detail(companyId, skillId), + new Promise((_, reject) => setTimeout(() => reject(new Error("skill detail timed out")), 1_000)), + ]); + + expect(detail?.usedByAgents).toEqual([ + expect.objectContaining({ + name: "Reviewer", + desired: true, + }), + ]); + expect(tracked.implicitCompanySkillSelects).not.toHaveBeenCalled(); + }); +}); diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index c1e564ddf5..97806ef8fc 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -886,11 +886,15 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { exitCode: 1, signal: null, timedOut: false, - errorCode: "codex_transient_upstream", + errorCode: "adapter_failed", + errorFamily: "transient_upstream", errorMessage: "Error running remote compact task: We're currently experiencing high demand, which may cause temporary errors.", provider: "openai", model: "gpt-5.4", + resultJson: { + errorFamily: "transient_upstream", + }, }); const { agentId, runId, issueId } = await seedQueuedIssueRunFixture(); @@ -911,7 +915,8 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { const failedRun = runs?.find((row) => row.id === runId); const retryRun = runs?.find((row) => row.id !== runId); expect(failedRun?.status).toBe("failed"); - expect(failedRun?.errorCode).toBe("codex_transient_upstream"); + expect(failedRun?.errorCode).toBe("adapter_failed"); + expect((failedRun?.resultJson as Record | null)?.errorFamily).toBe("transient_upstream"); expect(retryRun?.status).toBe("scheduled_retry"); expect(retryRun?.scheduledRetryReason).toBe("transient_failure"); expect((retryRun?.contextSnapshot as Record | null)?.codexTransientFallbackMode).toBe("same_session"); diff --git a/server/src/__tests__/heartbeat-retry-scheduling.test.ts b/server/src/__tests__/heartbeat-retry-scheduling.test.ts index 09ab2ec7a7..162ea539ed 100644 --- a/server/src/__tests__/heartbeat-retry-scheduling.test.ts +++ b/server/src/__tests__/heartbeat-retry-scheduling.test.ts @@ -56,8 +56,15 @@ describeEmbeddedPostgres("heartbeat bounded retry scheduling", () => { agentId: string; now: Date; errorCode: string; + errorFamily?: "transient_upstream" | null; + retryNotBefore?: string | null; scheduledRetryAttempt?: number; + resultJson?: Record | null; + adapterType?: "codex_local" | "claude_local"; + agentName?: string; }) { + const adapterType = input.adapterType ?? "codex_local"; + const agentName = input.agentName ?? (adapterType === "claude_local" ? "ClaudeCoder" : "CodexCoder"); await db.insert(companies).values({ id: input.companyId, name: "Paperclip", @@ -68,10 +75,10 @@ describeEmbeddedPostgres("heartbeat bounded retry scheduling", () => { await db.insert(agents).values({ id: input.agentId, companyId: input.companyId, - name: "CodexCoder", + name: agentName, role: "engineer", status: "active", - adapterType: "codex_local", + adapterType, adapterConfig: {}, runtimeConfig: { heartbeat: { @@ -93,6 +100,15 @@ describeEmbeddedPostgres("heartbeat bounded retry scheduling", () => { finishedAt: input.now, scheduledRetryAttempt: input.scheduledRetryAttempt ?? 0, scheduledRetryReason: input.scheduledRetryAttempt ? "transient_failure" : null, + resultJson: input.resultJson ?? { + ...(input.errorFamily ? { errorFamily: input.errorFamily } : {}), + ...(input.retryNotBefore + ? { + retryNotBefore: input.retryNotBefore, + transientRetryNotBefore: input.retryNotBefore, + } + : {}), + }, contextSnapshot: { issueId: randomUUID(), wakeReason: "issue_assigned", @@ -299,7 +315,8 @@ describeEmbeddedPostgres("heartbeat bounded retry scheduling", () => { companyId, agentId, now, - errorCode: "codex_transient_upstream", + errorCode: "adapter_failed", + errorFamily: "transient_upstream", scheduledRetryAttempt: index, }); @@ -335,4 +352,110 @@ describeEmbeddedPostgres("heartbeat bounded retry scheduling", () => { await db.delete(companies); } }); + + it("honors codex retry-not-before timestamps when they exceed the default bounded backoff", async () => { + const companyId = randomUUID(); + const agentId = randomUUID(); + const runId = randomUUID(); + const now = new Date(2026, 3, 22, 22, 29, 0); + const retryNotBefore = new Date(2026, 3, 22, 23, 31, 0); + + await seedRetryFixture({ + runId, + companyId, + agentId, + now, + errorCode: "adapter_failed", + errorFamily: "transient_upstream", + retryNotBefore: retryNotBefore.toISOString(), + }); + + const scheduled = await heartbeat.scheduleBoundedRetry(runId, { + now, + random: () => 0.5, + }); + + expect(scheduled.outcome).toBe("scheduled"); + if (scheduled.outcome !== "scheduled") return; + expect(scheduled.dueAt.getTime()).toBe(retryNotBefore.getTime()); + + const retryRun = await db + .select({ + contextSnapshot: heartbeatRuns.contextSnapshot, + scheduledRetryAt: heartbeatRuns.scheduledRetryAt, + wakeupRequestId: heartbeatRuns.wakeupRequestId, + }) + .from(heartbeatRuns) + .where(eq(heartbeatRuns.id, scheduled.run.id)) + .then((rows) => rows[0] ?? null); + + expect(retryRun?.scheduledRetryAt?.getTime()).toBe(retryNotBefore.getTime()); + expect((retryRun?.contextSnapshot as Record | null)?.transientRetryNotBefore).toBe( + retryNotBefore.toISOString(), + ); + + const wakeupRequest = await db + .select({ payload: agentWakeupRequests.payload }) + .from(agentWakeupRequests) + .where(eq(agentWakeupRequests.id, retryRun?.wakeupRequestId ?? "")) + .then((rows) => rows[0] ?? null); + + expect((wakeupRequest?.payload as Record | null)?.transientRetryNotBefore).toBe( + retryNotBefore.toISOString(), + ); + }); + + it("schedules bounded retries for claude_transient_upstream and honors its retry-not-before hint", async () => { + const companyId = randomUUID(); + const agentId = randomUUID(); + const runId = randomUUID(); + const now = new Date(2026, 3, 22, 10, 0, 0); + const retryNotBefore = new Date(2026, 3, 22, 16, 0, 0); + + await seedRetryFixture({ + runId, + companyId, + agentId, + now, + errorCode: "adapter_failed", + errorFamily: "transient_upstream", + adapterType: "claude_local", + retryNotBefore: retryNotBefore.toISOString(), + }); + + const scheduled = await heartbeat.scheduleBoundedRetry(runId, { + now, + random: () => 0.5, + }); + + expect(scheduled.outcome).toBe("scheduled"); + if (scheduled.outcome !== "scheduled") return; + expect(scheduled.dueAt.getTime()).toBe(retryNotBefore.getTime()); + + const retryRun = await db + .select({ + contextSnapshot: heartbeatRuns.contextSnapshot, + scheduledRetryAt: heartbeatRuns.scheduledRetryAt, + wakeupRequestId: heartbeatRuns.wakeupRequestId, + }) + .from(heartbeatRuns) + .where(eq(heartbeatRuns.id, scheduled.run.id)) + .then((rows) => rows[0] ?? null); + + expect(retryRun?.scheduledRetryAt?.getTime()).toBe(retryNotBefore.getTime()); + const contextSnapshot = (retryRun?.contextSnapshot as Record | null) ?? {}; + expect(contextSnapshot.transientRetryNotBefore).toBe(retryNotBefore.toISOString()); + // Claude does not participate in the Codex fallback-mode ladder. + expect(contextSnapshot.codexTransientFallbackMode ?? null).toBeNull(); + + const wakeupRequest = await db + .select({ payload: agentWakeupRequests.payload }) + .from(agentWakeupRequests) + .where(eq(agentWakeupRequests.id, retryRun?.wakeupRequestId ?? "")) + .then((rows) => rows[0] ?? null); + + expect((wakeupRequest?.payload as Record | null)?.transientRetryNotBefore).toBe( + retryNotBefore.toISOString(), + ); + }); }); diff --git a/server/src/__tests__/issue-comment-reopen-routes.test.ts b/server/src/__tests__/issue-comment-reopen-routes.test.ts index eddd32d43c..16748f6508 100644 --- a/server/src/__tests__/issue-comment-reopen-routes.test.ts +++ b/server/src/__tests__/issue-comment-reopen-routes.test.ts @@ -847,6 +847,9 @@ describe.sequential("issue comment reopen routes", () => { status: "in_review", assigneeAgentId: null, assigneeUserId: "local-board", + reviewRequest: { + instructions: "Please verify the fix against the reproduction steps and note any residual risk.", + }, }); expect(res.status).toBe(200); @@ -863,6 +866,9 @@ describe.sequential("issue comment reopen routes", () => { type: "agent", agentId: "22222222-2222-4222-8222-222222222222", }, + reviewRequest: { + instructions: "Please verify the fix against the reproduction steps and note any residual risk.", + }, }); await waitForWakeup(() => expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith( "33333333-3333-4333-8333-333333333333", @@ -873,6 +879,9 @@ describe.sequential("issue comment reopen routes", () => { executionStage: expect.objectContaining({ wakeRole: "reviewer", stageType: "review", + reviewRequest: { + instructions: "Please verify the fix against the reproduction steps and note any residual risk.", + }, allowedActions: ["approve", "request_changes"], }), }), diff --git a/server/src/__tests__/issue-execution-policy.test.ts b/server/src/__tests__/issue-execution-policy.test.ts index 6aeac5540f..37a64dc278 100644 --- a/server/src/__tests__/issue-execution-policy.test.ts +++ b/server/src/__tests__/issue-execution-policy.test.ts @@ -171,6 +171,75 @@ describe("issue execution policy transitions", () => { expect(result.decision).toBeUndefined(); }); + it("carries loose review instructions on the pending handoff", () => { + const reviewInstructions = [ + "Please focus on whether the migration path is reversible.", + "", + "- Check failure handling", + "- Call out any unclear operator instructions", + ].join("\n"); + + const result = applyIssueExecutionPolicyTransition({ + issue: { + status: "in_progress", + assigneeAgentId: coderAgentId, + assigneeUserId: null, + executionPolicy: policy, + executionState: null, + }, + policy, + requestedStatus: "done", + requestedAssigneePatch: {}, + actor: { agentId: coderAgentId }, + commentBody: "Implemented the migration", + reviewRequest: { instructions: reviewInstructions }, + }); + + expect(result.patch.executionState).toMatchObject({ + status: "pending", + currentStageType: "review", + currentParticipant: { type: "agent", agentId: qaAgentId }, + reviewRequest: { instructions: reviewInstructions }, + }); + }); + + it("clears loose review instructions with explicit null during a stage transition", () => { + const reviewStageId = policy.stages[0].id; + const result = applyIssueExecutionPolicyTransition({ + issue: { + status: "in_progress", + assigneeAgentId: coderAgentId, + assigneeUserId: null, + executionPolicy: policy, + executionState: { + status: "pending", + currentStageId: reviewStageId, + currentStageIndex: 0, + currentStageType: "review", + currentParticipant: { type: "agent", agentId: qaAgentId }, + returnAssignee: { type: "agent", agentId: coderAgentId }, + reviewRequest: { instructions: "Old review request" }, + completedStageIds: [], + lastDecisionId: null, + lastDecisionOutcome: null, + }, + }, + policy, + requestedStatus: "in_review", + requestedAssigneePatch: {}, + actor: { agentId: coderAgentId }, + commentBody: "Ready for review", + reviewRequest: null, + }); + + expect(result.patch.executionState).toMatchObject({ + status: "pending", + currentStageType: "review", + currentParticipant: { type: "agent", agentId: qaAgentId }, + reviewRequest: null, + }); + }); + it("reviewer approves → advances to approval stage", () => { const reviewStageId = policy.stages[0].id; const result = applyIssueExecutionPolicyTransition({ @@ -214,6 +283,44 @@ describe("issue execution policy transitions", () => { }); }); + it("lets a reviewer provide loose instructions for the next approval stage", () => { + const reviewStageId = policy.stages[0].id; + const approvalInstructions = "Please decide whether this is ready to ship, with any launch caveats."; + const result = applyIssueExecutionPolicyTransition({ + issue: { + status: "in_review", + assigneeAgentId: qaAgentId, + assigneeUserId: null, + executionPolicy: policy, + executionState: { + status: "pending", + currentStageId: reviewStageId, + currentStageIndex: 0, + currentStageType: "review", + currentParticipant: { type: "agent", agentId: qaAgentId }, + returnAssignee: { type: "agent", agentId: coderAgentId }, + reviewRequest: { instructions: "Review the implementation details." }, + completedStageIds: [], + lastDecisionId: null, + lastDecisionOutcome: null, + }, + }, + policy, + requestedStatus: "done", + requestedAssigneePatch: {}, + actor: { agentId: qaAgentId }, + commentBody: "QA signoff complete", + reviewRequest: { instructions: approvalInstructions }, + }); + + expect(result.patch.executionState).toMatchObject({ + status: "pending", + currentStageType: "approval", + currentParticipant: { type: "user", userId: ctoUserId }, + reviewRequest: { instructions: approvalInstructions }, + }); + }); + it("approver approves → marks completed (allows done)", () => { const reviewStageId = policy.stages[0].id; const approvalStageId = policy.stages[1].id; diff --git a/server/src/__tests__/issues-service.test.ts b/server/src/__tests__/issues-service.test.ts index b21c603e2e..05ea80c4e9 100644 --- a/server/src/__tests__/issues-service.test.ts +++ b/server/src/__tests__/issues-service.test.ts @@ -355,6 +355,110 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => { expect(result.map((issue) => issue.id)).toEqual([commentMatchId, descriptionMatchId]); }); + it("filters issue lists to the full descendant tree for a root issue", async () => { + const companyId = randomUUID(); + const rootId = randomUUID(); + const childId = randomUUID(); + const grandchildId = randomUUID(); + const siblingId = randomUUID(); + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + + await db.insert(issues).values([ + { + id: rootId, + companyId, + title: "Root", + status: "todo", + priority: "medium", + }, + { + id: childId, + companyId, + parentId: rootId, + title: "Child", + status: "todo", + priority: "medium", + }, + { + id: grandchildId, + companyId, + parentId: childId, + title: "Grandchild", + status: "todo", + priority: "medium", + }, + { + id: siblingId, + companyId, + title: "Sibling", + status: "todo", + priority: "medium", + }, + ]); + + const result = await svc.list(companyId, { descendantOf: rootId }); + + expect(new Set(result.map((issue) => issue.id))).toEqual(new Set([childId, grandchildId])); + }); + + it("combines descendant filtering with search", async () => { + const companyId = randomUUID(); + const rootId = randomUUID(); + const childId = randomUUID(); + const grandchildId = randomUUID(); + const outsideMatchId = randomUUID(); + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + + await db.insert(issues).values([ + { + id: rootId, + companyId, + title: "Root", + status: "todo", + priority: "medium", + }, + { + id: childId, + companyId, + parentId: rootId, + title: "Relevant parent", + status: "todo", + priority: "medium", + }, + { + id: grandchildId, + companyId, + parentId: childId, + title: "Needle grandchild", + status: "todo", + priority: "medium", + }, + { + id: outsideMatchId, + companyId, + title: "Needle outside", + status: "todo", + priority: "medium", + }, + ]); + + const result = await svc.list(companyId, { descendantOf: rootId, q: "needle" }); + + expect(result.map((issue) => issue.id)).toEqual([grandchildId]); + }); + it("accepts issue identifiers through getById", async () => { const companyId = randomUUID(); const issueId = randomUUID(); diff --git a/server/src/adapters/codex-models.ts b/server/src/adapters/codex-models.ts index 418bb6c76b..872779144f 100644 --- a/server/src/adapters/codex-models.ts +++ b/server/src/adapters/codex-models.ts @@ -70,14 +70,15 @@ async function fetchOpenAiModels(apiKey: string): Promise { } } -export async function listCodexModels(): Promise { +async function loadCodexModels(options?: { forceRefresh?: boolean }): Promise { + const forceRefresh = options?.forceRefresh === true; const apiKey = resolveOpenAiApiKey(); const fallback = dedupeModels(codexFallbackModels); if (!apiKey) return fallback; const now = Date.now(); const keyFingerprint = fingerprint(apiKey); - if (cached && cached.keyFingerprint === keyFingerprint && cached.expiresAt > now) { + if (!forceRefresh && cached && cached.keyFingerprint === keyFingerprint && cached.expiresAt > now) { return cached.models; } @@ -99,6 +100,14 @@ export async function listCodexModels(): Promise { return fallback; } +export async function listCodexModels(): Promise { + return loadCodexModels(); +} + +export async function refreshCodexModels(): Promise { + return loadCodexModels({ forceRefresh: true }); +} + export function resetCodexModelsCacheForTests() { cached = null; } diff --git a/server/src/adapters/index.ts b/server/src/adapters/index.ts index 49530dc772..9b27b32563 100644 --- a/server/src/adapters/index.ts +++ b/server/src/adapters/index.ts @@ -1,6 +1,7 @@ export { getServerAdapter, listAdapterModels, + refreshAdapterModels, listServerAdapters, findServerAdapter, findActiveServerAdapter, diff --git a/server/src/adapters/registry.ts b/server/src/adapters/registry.ts index 8268f0629a..20be46e11c 100644 --- a/server/src/adapters/registry.ts +++ b/server/src/adapters/registry.ts @@ -55,7 +55,7 @@ import { agentConfigurationDoc as openclawGatewayAgentConfigurationDoc, models as openclawGatewayModels, } from "@paperclipai/adapter-openclaw-gateway"; -import { listCodexModels } from "./codex-models.js"; +import { listCodexModels, refreshCodexModels } from "./codex-models.js"; import { listCursorModels } from "./cursor-models.js"; import { execute as piExecute, @@ -145,6 +145,7 @@ const codexLocalAdapter: ServerAdapterModule = { sessionManagement: getAdapterSessionManagement("codex_local") ?? undefined, models: codexModels, listModels: listCodexModels, + refreshModels: refreshCodexModels, supportsLocalAgentJwt: true, supportsInstructionsBundle: true, instructionsPathKey: "instructionsFilePath", @@ -459,6 +460,20 @@ export async function listAdapterModels(type: string): Promise<{ id: string; lab return adapter.models ?? []; } +export async function refreshAdapterModels(type: string): Promise<{ id: string; label: string }[]> { + const adapter = findActiveServerAdapter(type); + if (!adapter) return []; + if (adapter.refreshModels) { + const refreshed = await adapter.refreshModels(); + if (refreshed.length > 0) return refreshed; + } + if (adapter.listModels) { + const discovered = await adapter.listModels(); + if (discovered.length > 0) return discovered; + } + return adapter.models ?? []; +} + export function listServerAdapters(): ServerAdapterModule[] { return Array.from(adaptersByType.values()); } diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index 92e249eebe..ec204d2acf 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -59,6 +59,7 @@ import { findActiveServerAdapter, findServerAdapter, listAdapterModels, + refreshAdapterModels, requireServerAdapter, } from "../adapters/index.js"; import { redactEventPayload } from "../redaction.js"; @@ -877,7 +878,12 @@ export function agentRoutes(db: Db) { const companyId = req.params.companyId as string; assertCompanyAccess(req, companyId); const type = assertKnownAdapterType(req.params.type as string); - const models = await listAdapterModels(type); + const refresh = typeof req.query.refresh === "string" + ? ["1", "true", "yes"].includes(req.query.refresh.toLowerCase()) + : false; + const models = refresh + ? await refreshAdapterModels(type) + : await listAdapterModels(type); res.json(models); }); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index ef838e2c04..d09cff80c8 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -101,6 +101,7 @@ type ExecutionStageWakeContext = { stageType: ParsedExecutionState["currentStageType"]; currentParticipant: ParsedExecutionState["currentParticipant"]; returnAssignee: ParsedExecutionState["returnAssignee"]; + reviewRequest: ParsedExecutionState["reviewRequest"]; lastDecisionOutcome: ParsedExecutionState["lastDecisionOutcome"]; allowedActions: string[]; }; @@ -124,6 +125,7 @@ function buildExecutionStageWakeContext(input: { stageType: input.state.currentStageType, currentParticipant: input.state.currentParticipant, returnAssignee: input.state.returnAssignee, + reviewRequest: input.state.reviewRequest ?? null, lastDecisionOutcome: input.state.lastDecisionOutcome, allowedActions: input.allowedActions, }; @@ -833,6 +835,7 @@ export function issueRoutes( workspaceId: req.query.workspaceId as string | undefined, executionWorkspaceId: req.query.executionWorkspaceId as string | undefined, parentId: req.query.parentId as string | undefined, + descendantOf: req.query.descendantOf as string | undefined, labelId: req.query.labelId as string | undefined, originKind: req.query.originKind as string | undefined, originId: req.query.originId as string | undefined, @@ -1789,6 +1792,7 @@ export function issueRoutes( : null; const { comment: commentBody, + reviewRequest, reopen: reopenRequested, interrupt: interruptRequested, hiddenAt: hiddenAtRaw, @@ -1815,7 +1819,8 @@ export function issueRoutes( : false; let interruptedRunId: string | null = null; const closedExecutionWorkspace = await getClosedIssueExecutionWorkspace(existing); - const isAgentWorkUpdate = req.actor.type === "agent" && Object.keys(updateFields).length > 0; + const isAgentWorkUpdate = + req.actor.type === "agent" && (Object.keys(updateFields).length > 0 || reviewRequest !== undefined); if (closedExecutionWorkspace && (commentBody || isAgentWorkUpdate)) { respondClosedIssueExecutionWorkspace(res, closedExecutionWorkspace); @@ -1889,6 +1894,7 @@ export function issueRoutes( userId: actor.actorType === "user" ? actor.actorId : null, }, commentBody, + reviewRequest: reviewRequest === undefined ? undefined : reviewRequest, }); const decisionId = transition.decision ? randomUUID() : null; if (decisionId) { @@ -1902,6 +1908,20 @@ export function issueRoutes( }; } Object.assign(updateFields, transition.patch); + if (reviewRequest !== undefined && transition.patch.executionState === undefined) { + const existingExecutionState = parseIssueExecutionState(existing.executionState); + if (!existingExecutionState || existingExecutionState.status !== "pending") { + if (reviewRequest !== null) { + res.status(422).json({ error: "reviewRequest requires an active review or approval stage" }); + return; + } + } else { + updateFields.executionState = { + ...existingExecutionState, + reviewRequest, + }; + } + } const nextAssigneeAgentId = updateFields.assigneeAgentId === undefined ? existing.assigneeAgentId : (updateFields.assigneeAgentId as string | null); diff --git a/server/src/services/companies.ts b/server/src/services/companies.ts index 89678572ca..9220081ef0 100644 --- a/server/src/services/companies.ts +++ b/server/src/services/companies.ts @@ -27,6 +27,7 @@ import { principalPermissionGrants, companyMemberships, companySkills, + documents, } from "@paperclipai/db"; import { notFound, unprocessable } from "../errors.js"; @@ -279,6 +280,7 @@ export function companyService(db: Db) { await tx.delete(companyMemberships).where(eq(companyMemberships.companyId, id)); await tx.delete(companySkills).where(eq(companySkills.companyId, id)); await tx.delete(issueReadStates).where(eq(issueReadStates.companyId, id)); + await tx.delete(documents).where(eq(documents.companyId, id)); await tx.delete(issues).where(eq(issues.companyId, id)); await tx.delete(companyLogos).where(eq(companyLogos.companyId, id)); await tx.delete(assets).where(eq(assets.companyId, id)); diff --git a/server/src/services/company-skills.ts b/server/src/services/company-skills.ts index 0985c2b622..8c67a0c3a1 100644 --- a/server/src/services/company-skills.ts +++ b/server/src/services/company-skills.ts @@ -27,13 +27,11 @@ import type { CompanySkillUsageAgent, } from "@paperclipai/shared"; import { normalizeAgentUrlKey } from "@paperclipai/shared"; -import { findActiveServerAdapter } from "../adapters/index.js"; import { resolvePaperclipInstanceRoot } from "../home-paths.js"; import { notFound, unprocessable } from "../errors.js"; import { ghFetch, gitHubApiBase, resolveRawGitHubUrl } from "./github-fetch.js"; import { agentService } from "./agents.js"; import { projectService } from "./projects.js"; -import { secretService } from "./secrets.js"; type CompanySkillRow = typeof companySkills.$inferSelect; type CompanySkillListDbRow = Pick< @@ -72,6 +70,12 @@ type CompanySkillListRow = Pick< | "createdAt" | "updatedAt" >; +type CompanySkillReferenceRow = Pick< + CompanySkillRow, + | "id" + | "key" + | "slug" +>; type SkillReferenceTarget = Pick; type SkillSourceInfoTarget = Pick< CompanySkill, @@ -147,6 +151,27 @@ type RuntimeSkillEntryOptions = { const skillInventoryRefreshPromises = new Map>(); +function selectCompanySkillColumns() { + return { + id: companySkills.id, + companyId: companySkills.companyId, + key: companySkills.key, + slug: companySkills.slug, + name: companySkills.name, + description: companySkills.description, + markdown: companySkills.markdown, + sourceType: companySkills.sourceType, + sourceLocator: companySkills.sourceLocator, + sourceRef: companySkills.sourceRef, + trustLevel: companySkills.trustLevel, + compatibility: companySkills.compatibility, + fileInventory: companySkills.fileInventory, + metadata: companySkills.metadata, + createdAt: companySkills.createdAt, + updatedAt: companySkills.updatedAt, + }; +} + const PROJECT_SCAN_DIRECTORY_ROOTS = [ "skills", "skills/.curated", @@ -1523,7 +1548,6 @@ function toCompanySkillListItem(skill: CompanySkillListRow, attachedAgentCount: export function companySkillService(db: Db) { const agents = agentService(db); const projects = projectService(db); - const secretsSvc = secretService(db); async function ensureBundledSkills(companyId: string) { for (const skillsRoot of resolveBundledSkillsRoot()) { @@ -1553,10 +1577,19 @@ export function companySkillService(db: Db) { async function pruneMissingLocalPathSkills(companyId: string) { const rows = await db - .select() + .select({ + id: companySkills.id, + key: companySkills.key, + slug: companySkills.slug, + sourceType: companySkills.sourceType, + sourceLocator: companySkills.sourceLocator, + }) .from(companySkills) .where(eq(companySkills.companyId, companyId)); - const skills = rows.map((row) => toCompanySkill(row)); + const skills = rows.map((row) => ({ + ...row, + sourceType: row.sourceType as CompanySkillSourceType, + })); const missingIds = new Set(await findMissingLocalSkillIds(skills)); if (missingIds.size === 0) return; @@ -1628,25 +1661,37 @@ export function companySkillService(db: Db) { async function listFull(companyId: string): Promise { await ensureSkillInventoryCurrent(companyId); const rows = await db - .select() + .select(selectCompanySkillColumns()) .from(companySkills) .where(eq(companySkills.companyId, companyId)) .orderBy(asc(companySkills.name), asc(companySkills.key)); return rows.map((row) => toCompanySkill(row)); } - async function getById(id: string) { - const row = await db - .select() + async function listReferenceTargets(companyId: string): Promise { + const rows = await db + .select({ + id: companySkills.id, + key: companySkills.key, + slug: companySkills.slug, + }) .from(companySkills) - .where(eq(companySkills.id, id)) + .where(eq(companySkills.companyId, companyId)); + return rows as CompanySkillReferenceRow[]; + } + + async function getById(companyId: string, id: string) { + const row = await db + .select(selectCompanySkillColumns()) + .from(companySkills) + .where(and(eq(companySkills.companyId, companyId), eq(companySkills.id, id))) .then((rows) => rows[0] ?? null); return row ? toCompanySkill(row) : null; } async function getByKey(companyId: string, key: string) { const row = await db - .select() + .select(selectCompanySkillColumns()) .from(companySkills) .where(and(eq(companySkills.companyId, companyId), eq(companySkills.key, key))) .then((rows) => rows[0] ?? null); @@ -1654,67 +1699,36 @@ export function companySkillService(db: Db) { } async function usage(companyId: string, key: string): Promise { - const skills = await listFull(companyId); + const skills = await listReferenceTargets(companyId); const agentRows = await agents.list(companyId); const desiredAgents = agentRows.filter((agent) => { const desiredSkills = resolveDesiredSkillKeys(skills, agent.adapterConfig as Record); return desiredSkills.includes(key); }); - return Promise.all( - desiredAgents.map(async (agent) => { - const adapter = findActiveServerAdapter(agent.adapterType); - let actualState: string | null = null; - - if (!adapter?.listSkills) { - actualState = "unsupported"; - } else { - try { - const { config: runtimeConfig } = await secretsSvc.resolveAdapterConfigForRuntime( - agent.companyId, - agent.adapterConfig as Record, - ); - const runtimeSkillEntries = await listRuntimeSkillEntries(agent.companyId); - const snapshot = await adapter.listSkills({ - agentId: agent.id, - companyId: agent.companyId, - adapterType: agent.adapterType, - config: { - ...runtimeConfig, - paperclipRuntimeSkills: runtimeSkillEntries, - }, - }); - actualState = snapshot.entries.find((entry) => entry.key === key)?.state - ?? (snapshot.supported ? "missing" : "unsupported"); - } catch { - actualState = "unknown"; - } - } - - return { - id: agent.id, - name: agent.name, - urlKey: agent.urlKey, - adapterType: agent.adapterType, - desired: true, - actualState, - }; - }), - ); + return desiredAgents.map((agent) => ({ + id: agent.id, + name: agent.name, + urlKey: agent.urlKey, + adapterType: agent.adapterType, + desired: true, + // Runtime adapter state is intentionally omitted from this bounded metadata read. + actualState: null, + })); } async function detail(companyId: string, id: string): Promise { await ensureSkillInventoryCurrent(companyId); - const skill = await getById(id); - if (!skill || skill.companyId !== companyId) return null; + const skill = await getById(companyId, id); + if (!skill) return null; const usedByAgents = await usage(companyId, skill.key); return enrichSkill(skill, usedByAgents.length, usedByAgents); } async function updateStatus(companyId: string, skillId: string): Promise { await ensureSkillInventoryCurrent(companyId); - const skill = await getById(skillId); - if (!skill || skill.companyId !== companyId) return null; + const skill = await getById(companyId, skillId); + if (!skill) return null; if (skill.sourceType !== "github" && skill.sourceType !== "skills_sh") { return { @@ -1757,8 +1771,8 @@ export function companySkillService(db: Db) { async function readFile(companyId: string, skillId: string, relativePath: string): Promise { await ensureSkillInventoryCurrent(companyId); - const skill = await getById(skillId); - if (!skill || skill.companyId !== companyId) return null; + const skill = await getById(companyId, skillId); + if (!skill) return null; const normalizedPath = normalizePortablePath(relativePath || "SKILL.md"); const fileEntry = skill.fileInventory.find((entry) => entry.path === normalizedPath); @@ -1855,8 +1869,8 @@ export function companySkillService(db: Db) { async function updateFile(companyId: string, skillId: string, relativePath: string, content: string): Promise { await ensureSkillInventoryCurrent(companyId); - const skill = await getById(skillId); - if (!skill || skill.companyId !== companyId) throw notFound("Skill not found"); + const skill = await getById(companyId, skillId); + if (!skill) throw notFound("Skill not found"); const source = deriveSkillSourceInfo(skill); if (!source.editable || skill.sourceType !== "local_path") { @@ -1895,8 +1909,8 @@ export function companySkillService(db: Db) { async function installUpdate(companyId: string, skillId: string): Promise { await ensureSkillInventoryCurrent(companyId); - const skill = await getById(skillId); - if (!skill || skill.companyId !== companyId) return null; + const skill = await getById(companyId, skillId); + if (!skill) return null; const status = await updateStatus(companyId, skillId); if (!status?.supported) { @@ -2136,7 +2150,7 @@ export function companySkillService(db: Db) { return skillDir; } - function resolveRuntimeSkillMaterializedPath(companyId: string, skill: CompanySkill) { + function resolveRuntimeSkillMaterializedPath(companyId: string, skill: Pick) { const runtimeRoot = path.resolve(resolveManagedSkillsRoot(companyId), "__runtime__"); return path.resolve(runtimeRoot, buildSkillRuntimeName(skill.key, skill.slug)); } diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 1ee91d57aa..e9fe107ab1 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -182,6 +182,61 @@ function resolveCodexTransientFallbackMode(attempt: number): CodexTransientFallb if (attempt === 3) return "fresh_session"; return "fresh_session_safer_invocation"; } + +function readHeartbeatRunErrorFamily( + run: Pick, +) { + const resultJson = parseObject(run.resultJson); + const persistedFamily = readNonEmptyString(resultJson.errorFamily); + if (persistedFamily) return persistedFamily; + + if (run.errorCode === "codex_transient_upstream" || run.errorCode === "claude_transient_upstream") { + return "transient_upstream"; + } + return null; +} + +function readTransientRetryNotBeforeFromRun(run: Pick) { + const resultJson = parseObject(run.resultJson); + const value = resultJson.retryNotBefore ?? resultJson.transientRetryNotBefore; + if (!(typeof value === "string" || typeof value === "number" || value instanceof Date)) { + return null; + } + const parsed = new Date(value); + return Number.isNaN(parsed.getTime()) ? null : parsed; +} + +function readTransientRecoveryContractFromRun( + run: Pick, +) { + return readHeartbeatRunErrorFamily(run) === "transient_upstream" + ? { + errorFamily: "transient_upstream" as const, + retryNotBefore: readTransientRetryNotBeforeFromRun(run), + } + : null; +} + +function mergeAdapterRecoveryMetadata(input: { + resultJson: Record | null | undefined; + errorFamily?: string | null; + retryNotBefore?: string | null; +}) { + const errorFamily = readNonEmptyString(input.errorFamily); + const retryNotBefore = readNonEmptyString(input.retryNotBefore); + if (!input.resultJson && !errorFamily && !retryNotBefore) return input.resultJson ?? null; + + return { + ...(input.resultJson ?? {}), + ...(errorFamily ? { errorFamily } : {}), + ...(retryNotBefore + ? { + retryNotBefore, + transientRetryNotBefore: retryNotBefore, + } + : {}), + }; +} const RUNNING_ISSUE_WAKE_REASONS_REQUIRING_FOLLOWUP = new Set(["approval_approved"]); const SESSIONED_LOCAL_ADAPTERS = new Set([ "claude_local", @@ -3281,13 +3336,18 @@ export function heartbeatService(db: Db) { const retryReason = opts?.retryReason ?? BOUNDED_TRANSIENT_HEARTBEAT_RETRY_REASON; const wakeReason = opts?.wakeReason ?? BOUNDED_TRANSIENT_HEARTBEAT_RETRY_WAKE_REASON; const nextAttempt = (run.scheduledRetryAttempt ?? 0) + 1; - const schedule = computeBoundedTransientHeartbeatRetrySchedule(nextAttempt, now, opts?.random); + const baseSchedule = computeBoundedTransientHeartbeatRetrySchedule(nextAttempt, now, opts?.random); + const transientRecovery = + retryReason === BOUNDED_TRANSIENT_HEARTBEAT_RETRY_REASON + ? readTransientRecoveryContractFromRun(run) + : null; const codexTransientFallbackMode = - agent.adapterType === "codex_local" && retryReason === BOUNDED_TRANSIENT_HEARTBEAT_RETRY_REASON && run.errorCode === "codex_transient_upstream" + agent.adapterType === "codex_local" && transientRecovery ? resolveCodexTransientFallbackMode(nextAttempt) : null; + const transientRetryNotBefore = transientRecovery?.retryNotBefore ?? null; - if (!schedule) { + if (!baseSchedule) { await appendRunEvent(run, await nextRunEventSeq(run.id), { eventType: "lifecycle", stream: "system", @@ -3305,6 +3365,14 @@ export function heartbeatService(db: Db) { maxAttempts: BOUNDED_TRANSIENT_HEARTBEAT_RETRY_MAX_ATTEMPTS, }; } + const schedule = + transientRetryNotBefore && transientRetryNotBefore.getTime() > baseSchedule.dueAt.getTime() + ? { + ...baseSchedule, + dueAt: transientRetryNotBefore, + delayMs: Math.max(0, transientRetryNotBefore.getTime() - now.getTime()), + } + : baseSchedule; const contextSnapshot = parseObject(run.contextSnapshot); const issueId = readNonEmptyString(contextSnapshot.issueId); @@ -3315,8 +3383,10 @@ export function heartbeatService(db: Db) { retryOfRunId: run.id, wakeReason, retryReason, + ...(transientRecovery ? { errorFamily: transientRecovery.errorFamily } : {}), scheduledRetryAttempt: schedule.attempt, scheduledRetryAt: schedule.dueAt.toISOString(), + ...(transientRetryNotBefore ? { transientRetryNotBefore: transientRetryNotBefore.toISOString() } : {}), ...(codexTransientFallbackMode ? { codexTransientFallbackMode } : {}), }; @@ -3333,8 +3403,10 @@ export function heartbeatService(db: Db) { ...(issueId ? { issueId } : {}), retryOfRunId: run.id, retryReason, + ...(transientRecovery ? { errorFamily: transientRecovery.errorFamily } : {}), scheduledRetryAttempt: schedule.attempt, scheduledRetryAt: schedule.dueAt.toISOString(), + ...(transientRetryNotBefore ? { transientRetryNotBefore: transientRetryNotBefore.toISOString() } : {}), ...(codexTransientFallbackMode ? { codexTransientFallbackMode } : {}), }, status: "queued", @@ -3397,10 +3469,12 @@ export function heartbeatService(db: Db) { payload: { retryRunId: retryRun.id, retryReason, + ...(transientRecovery ? { errorFamily: transientRecovery.errorFamily } : {}), scheduledRetryAttempt: schedule.attempt, scheduledRetryAt: schedule.dueAt.toISOString(), baseDelayMs: schedule.baseDelayMs, delayMs: schedule.delayMs, + ...(transientRetryNotBefore ? { transientRetryNotBefore: transientRetryNotBefore.toISOString() } : {}), ...(codexTransientFallbackMode ? { codexTransientFallbackMode } : {}), }, }); @@ -5223,7 +5297,11 @@ export function heartbeatService(db: Db) { const persistedResultJson = mergeHeartbeatRunResultJson( mergeRunStopMetadataForAgent(agent, outcome, { - resultJson: adapterResult.resultJson ?? null, + resultJson: mergeAdapterRecoveryMetadata({ + resultJson: adapterResult.resultJson ?? null, + errorFamily: adapterResult.errorFamily ?? null, + retryNotBefore: adapterResult.retryNotBefore ?? null, + }), errorCode: runErrorCode, errorMessage: runErrorMessage, }), @@ -5284,7 +5362,7 @@ export function heartbeatService(db: Db) { ); } } - if (outcome === "failed" && livenessRun.errorCode === "codex_transient_upstream") { + if (outcome === "failed" && readTransientRecoveryContractFromRun(livenessRun)) { await scheduleBoundedRetryForRun(livenessRun, agent); } await finalizeIssueCommentPolicy(livenessRun, agent); @@ -5618,6 +5696,8 @@ export function heartbeatService(db: Db) { } const deferredCommentIds = extractWakeCommentIds(deferredContextSeed); const deferredWakeReason = readNonEmptyString(deferredContextSeed.wakeReason); + // Only human/comment-reopen interactions should revive completed issues; + // system follow-ups such as retry or cleanup wakes must not reopen closed work. const shouldReopenDeferredCommentWake = deferredCommentIds.length > 0 && (issue.status === "done" || issue.status === "cancelled") && diff --git a/server/src/services/issue-execution-policy.ts b/server/src/services/issue-execution-policy.ts index 428a11013d..9a78512d18 100644 --- a/server/src/services/issue-execution-policy.ts +++ b/server/src/services/issue-execution-policy.ts @@ -31,6 +31,7 @@ type TransitionInput = { requestedAssigneePatch: RequestedAssigneePatch; actor: ActorLike; commentBody?: string | null; + reviewRequest?: IssueExecutionState["reviewRequest"] | null; }; type TransitionResult = { @@ -168,6 +169,7 @@ function buildCompletedState(previous: IssueExecutionState | null, currentStage: currentStageType: null, currentParticipant: null, returnAssignee: previous?.returnAssignee ?? null, + reviewRequest: null, completedStageIds, lastDecisionId: previous?.lastDecisionId ?? null, lastDecisionOutcome: "approved", @@ -186,6 +188,7 @@ function buildStateWithCompletedStages(input: { currentStageType: input.previous?.currentStageType ?? null, currentParticipant: input.previous?.currentParticipant ?? null, returnAssignee: input.previous?.returnAssignee ?? input.returnAssignee, + reviewRequest: input.previous?.reviewRequest ?? null, completedStageIds: input.completedStageIds, lastDecisionId: input.previous?.lastDecisionId ?? null, lastDecisionOutcome: input.previous?.lastDecisionOutcome ?? null, @@ -204,6 +207,7 @@ function buildSkippedStageCompletedState(input: { currentStageType: null, currentParticipant: null, returnAssignee: input.previous?.returnAssignee ?? input.returnAssignee, + reviewRequest: null, completedStageIds: input.completedStageIds, lastDecisionId: input.previous?.lastDecisionId ?? null, lastDecisionOutcome: input.previous?.lastDecisionOutcome ?? null, @@ -216,6 +220,7 @@ function buildPendingState(input: { stageIndex: number; participant: IssueExecutionStagePrincipal; returnAssignee: IssueExecutionStagePrincipal | null; + reviewRequest?: IssueExecutionState["reviewRequest"] | null; }): IssueExecutionState { return { status: PENDING_STATUS, @@ -224,6 +229,7 @@ function buildPendingState(input: { currentStageType: input.stage.type, currentParticipant: input.participant, returnAssignee: input.returnAssignee, + reviewRequest: input.reviewRequest ?? null, completedStageIds: input.previous?.completedStageIds ?? [], lastDecisionId: input.previous?.lastDecisionId ?? null, lastDecisionOutcome: input.previous?.lastDecisionOutcome ?? null, @@ -236,6 +242,7 @@ function buildChangesRequestedState(previous: IssueExecutionState, currentStage: status: CHANGES_REQUESTED_STATUS, currentStageId: currentStage.id, currentStageType: currentStage.type, + reviewRequest: null, lastDecisionOutcome: "changes_requested", }; } @@ -247,6 +254,7 @@ function buildPendingStagePatch(input: { stage: IssueExecutionStage; participant: IssueExecutionStagePrincipal; returnAssignee: IssueExecutionStagePrincipal | null; + reviewRequest?: IssueExecutionState["reviewRequest"] | null; }) { input.patch.status = "in_review"; Object.assign(input.patch, patchForPrincipal(input.participant)); @@ -256,6 +264,7 @@ function buildPendingStagePatch(input: { stageIndex: input.policy.stages.findIndex((candidate) => candidate.id === input.stage.id), participant: input.participant, returnAssignee: input.returnAssignee, + reviewRequest: input.reviewRequest, }); } @@ -295,6 +304,9 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra const currentStage = input.policy ? findStageById(input.policy, existingState?.currentStageId) : null; const requestedStatus = input.requestedStatus; const activeStage = currentStage && existingState?.status === PENDING_STATUS ? currentStage : null; + const effectiveReviewRequest = input.reviewRequest === undefined + ? existingState?.reviewRequest ?? null + : input.reviewRequest; if (!input.policy) { if (existingState) { @@ -359,6 +371,7 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra stage: activeStage, participant, returnAssignee: existingState?.returnAssignee ?? currentAssignee ?? actor, + reviewRequest: effectiveReviewRequest, }); return { patch, @@ -405,6 +418,7 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra stage: nextStage, participant, returnAssignee: existingState?.returnAssignee ?? currentAssignee ?? actor, + reviewRequest: input.reviewRequest ?? null, }); return { patch, @@ -461,6 +475,7 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra stage: activeStage, participant: currentParticipant, returnAssignee: existingState?.returnAssignee ?? currentAssignee ?? actor, + reviewRequest: effectiveReviewRequest, }); return { patch, @@ -538,6 +553,7 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra stage: pendingStage, participant, returnAssignee, + reviewRequest: input.reviewRequest ?? null, }); return { patch, diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index 013d0e9b51..36e9ac97af 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -106,6 +106,7 @@ export interface IssueFilters { workspaceId?: string; executionWorkspaceId?: string; parentId?: string; + descendantOf?: string; labelId?: string; originKind?: string; originId?: string; @@ -1396,6 +1397,24 @@ export function issueService(db: Db) { AND ${issueComments.body} ILIKE ${containsPattern} ESCAPE '\\' ) `; + if (filters?.descendantOf) { + conditions.push(sql` + ${issues.id} IN ( + WITH RECURSIVE descendants(id) AS ( + SELECT ${issues.id} + FROM ${issues} + WHERE ${issues.companyId} = ${companyId} + AND ${issues.parentId} = ${filters.descendantOf} + UNION + SELECT ${issues.id} + FROM ${issues} + JOIN descendants ON ${issues.parentId} = descendants.id + WHERE ${issues.companyId} = ${companyId} + ) + SELECT id FROM descendants + ) + `); + } if (filters?.status) { const statuses = filters.status.split(",").map((s) => s.trim()); conditions.push(statuses.length === 1 ? eq(issues.status, statuses[0]) : inArray(issues.status, statuses)); diff --git a/ui/src/api/agents.ts b/ui/src/api/agents.ts index 9f7c8ba87f..0091be1d27 100644 --- a/ui/src/api/agents.ts +++ b/ui/src/api/agents.ts @@ -164,9 +164,9 @@ export const agentsApi = { api.get(agentPath(id, companyId, "/task-sessions")), resetSession: (id: string, taskKey?: string | null, companyId?: string) => api.post(agentPath(id, companyId, "/runtime-state/reset-session"), { taskKey: taskKey ?? null }), - adapterModels: (companyId: string, type: string) => + adapterModels: (companyId: string, type: string, options?: { refresh?: boolean }) => api.get( - `/companies/${encodeURIComponent(companyId)}/adapters/${encodeURIComponent(type)}/models`, + `/companies/${encodeURIComponent(companyId)}/adapters/${encodeURIComponent(type)}/models${options?.refresh ? "?refresh=1" : ""}`, ), detectModel: (companyId: string, type: string) => api.get( diff --git a/ui/src/api/issues.test.ts b/ui/src/api/issues.test.ts index a32add42e6..dedbfa9cd7 100644 --- a/ui/src/api/issues.test.ts +++ b/ui/src/api/issues.test.ts @@ -24,6 +24,14 @@ describe("issuesApi.list", () => { ); }); + it("passes descendantOf through to the company issues endpoint", async () => { + await issuesApi.list("company-1", { descendantOf: "issue-root-1", limit: 25 }); + + expect(mockApi.get).toHaveBeenCalledWith( + "/companies/company-1/issues?descendantOf=issue-root-1&limit=25", + ); + }); + it("passes generic workspaceId filters through to the company issues endpoint", async () => { await issuesApi.list("company-1", { workspaceId: "workspace-1", limit: 1000 }); diff --git a/ui/src/api/issues.ts b/ui/src/api/issues.ts index b285651374..d7e244b25c 100644 --- a/ui/src/api/issues.ts +++ b/ui/src/api/issues.ts @@ -43,6 +43,7 @@ export const issuesApi = { executionWorkspaceId?: string; originKind?: string; originId?: string; + descendantOf?: string; includeRoutineExecutions?: boolean; q?: string; limit?: number; @@ -63,6 +64,7 @@ export const issuesApi = { if (filters?.executionWorkspaceId) params.set("executionWorkspaceId", filters.executionWorkspaceId); if (filters?.originKind) params.set("originKind", filters.originKind); if (filters?.originId) params.set("originId", filters.originId); + if (filters?.descendantOf) params.set("descendantOf", filters.descendantOf); if (filters?.includeRoutineExecutions) params.set("includeRoutineExecutions", "true"); if (filters?.q) params.set("q", filters.q); if (filters?.limit) params.set("limit", String(filters.limit)); diff --git a/ui/src/components/AgentConfigForm.tsx b/ui/src/components/AgentConfigForm.tsx index c786182063..a63a3b4ad5 100644 --- a/ui/src/components/AgentConfigForm.tsx +++ b/ui/src/components/AgentConfigForm.tsx @@ -302,16 +302,19 @@ export function AgentConfigForm(props: AgentConfigFormProps) { ); // Fetch adapter models for the effective adapter type + const modelQueryKey = selectedCompanyId + ? queryKeys.agents.adapterModels(selectedCompanyId, adapterType) + : ["agents", "none", "adapter-models", adapterType]; const { data: fetchedModels, error: fetchedModelsError, } = useQuery({ - queryKey: selectedCompanyId - ? queryKeys.agents.adapterModels(selectedCompanyId, adapterType) - : ["agents", "none", "adapter-models", adapterType], + queryKey: modelQueryKey, queryFn: () => agentsApi.adapterModels(selectedCompanyId!, adapterType), enabled: Boolean(selectedCompanyId), }); + const [refreshModelsError, setRefreshModelsError] = useState(null); + const [refreshingModels, setRefreshingModels] = useState(false); const models = fetchedModels ?? externalModels ?? []; const adapterCommandField = adapterType === "hermes_local" ? "hermesCommand" : "command"; @@ -401,6 +404,20 @@ export function AgentConfigForm(props: AgentConfigFormProps) { ? val!.model : eff("adapterConfig", "model", String(config.model ?? "")); + async function handleRefreshModels() { + if (!selectedCompanyId) return; + setRefreshingModels(true); + setRefreshModelsError(null); + try { + const refreshed = await agentsApi.adapterModels(selectedCompanyId, adapterType, { refresh: true }); + queryClient.setQueryData(modelQueryKey, refreshed); + } catch (error) { + setRefreshModelsError(error instanceof Error ? error.message : "Failed to refresh adapter models."); + } finally { + setRefreshingModels(false); + } + } + const thinkingEffortKey = adapterType === "codex_local" ? "modelReasoningEffort" @@ -792,14 +809,17 @@ export function AgentConfigForm(props: AgentConfigFormProps) { const result = await refetchDetectedModel(); return result.data?.model ?? null; }} + onRefreshModels={adapterType === "codex_local" ? handleRefreshModels : undefined} + refreshingModels={refreshingModels} detectModelLabel="Detect model" emptyDetectHint="No model detected. Select or enter one manually." /> - {fetchedModelsError && ( + {(refreshModelsError || fetchedModelsError) && (

- {fetchedModelsError instanceof Error - ? fetchedModelsError.message - : "Failed to load adapter models."} + {refreshModelsError + ?? (fetchedModelsError instanceof Error + ? fetchedModelsError.message + : "Failed to load adapter models.")}

)} @@ -1134,6 +1154,8 @@ function ModelDropdown({ detectedModel, detectedModelCandidates, onDetectModel, + onRefreshModels, + refreshingModels, detectModelLabel, emptyDetectHint, }: { @@ -1149,6 +1171,8 @@ function ModelDropdown({ detectedModel?: string | null; detectedModelCandidates?: string[]; onDetectModel?: () => Promise; + onRefreshModels?: () => Promise; + refreshingModels?: boolean; detectModelLabel?: string; emptyDetectHint?: string; }) { @@ -1280,6 +1304,24 @@ function ModelDropdown({ {detectingModel ? "Detecting..." : detectedModel ? (detectModelLabel?.replace(/^Detect\b/, "Re-detect") ?? "Re-detect from config") : (detectModelLabel ?? "Detect from config")} )} + {onRefreshModels && !modelSearch.trim() && ( + + )} {value && (!models.some((m) => m.id === value) || promotedModelIds.has(value)) && ( + + {expanded ? ( +

+ {summary} +

+ ) : null} + + ); + + return ( +
+ {isCurrentUser ? ( +
+ {rowContent} +
+ ) : ( +
+ + {actorIcon ? ( + + ) : ( + {initialsForName(actorName)} + )} + + {rowContent} +
+ )} + {expanded ? ( +
+ +
+ ) : null} +
+ ); +} + function IssueChatSystemMessage({ message }: { message: ThreadMessage }) { const { agentMap, @@ -1767,6 +1890,16 @@ function IssueChatSystemMessage({ message }: { message: ThreadMessage }) { : null; if (custom.kind === "interaction" && interaction) { + if (interaction.kind === "request_confirmation" && interaction.status === "expired") { + return ( + + ); + } + return (
@@ -1921,12 +2054,15 @@ const IssueChatComposer = forwardRef(null); const editorRef = useRef(null); const composerContainerRef = useRef(null); const draftTimer = useRef | null>(null); + const canAcceptFiles = Boolean(onImageUpload || onAttachImage); function queueViewportRestore(snapshot: ReturnType) { if (!snapshot) return; @@ -2026,25 +2162,46 @@ const IssueChatComposer = forwardRef prev ? `${prev}\n\n${markdown}` : markdown); + } else if (onAttachImage) { + await onAttachImage(file); + } + } + async function handleAttachFile(evt: ChangeEvent) { const file = evt.target.files?.[0]; if (!file) return; setAttaching(true); try { - if (onImageUpload) { - const url = await onImageUpload(file); - const safeName = file.name.replace(/[[\]]/g, "\\$&"); - const markdown = `![${safeName}](${url})`; - setBody((prev) => prev ? `${prev}\n\n${markdown}` : markdown); - } else if (onAttachImage) { - await onAttachImage(file); - } + await attachFile(file); } finally { setAttaching(false); if (attachInputRef.current) attachInputRef.current.value = ""; } } + async function handleDroppedFiles(files: FileList | null | undefined) { + if (!files || files.length === 0) return; + setAttaching(true); + try { + for (const file of Array.from(files)) { + await attachFile(file); + } + } finally { + setAttaching(false); + } + } + + function resetDragState() { + dragDepthRef.current = 0; + setIsDragOver(false); + } + const canSubmit = !submitting && !!body.trim(); if (composerDisabledReason) { @@ -2059,7 +2216,35 @@ const IssueChatComposer = forwardRef { + if (!canAcceptFiles || !hasFilePayload(evt)) return; + dragDepthRef.current += 1; + setIsDragOver(true); + }} + onDragOver={(evt) => { + if (!canAcceptFiles || !hasFilePayload(evt)) return; + evt.preventDefault(); + evt.dataTransfer.dropEffect = "copy"; + }} + onDragLeave={() => { + if (!canAcceptFiles) return; + dragDepthRef.current = Math.max(0, dragDepthRef.current - 1); + if (dragDepthRef.current === 0) setIsDragOver(false); + }} + onDrop={(evt) => { + if (!canAcceptFiles) return; + if (evt.defaultPrevented) { + resetDragState(); + return; + } + evt.preventDefault(); + resetDragState(); + void handleDroppedFiles(evt.dataTransfer?.files); + }} > {composerHint ? ( @@ -2204,6 +2389,11 @@ export function IssueChatThread({ const composerViewportAnchorRef = useRef(null); const composerViewportSnapshotRef = useRef>(null); const preserveComposerViewportRef = useRef(false); + const pendingSubmitScrollRef = useRef(false); + const lastUserMessageIdRef = useRef(null); + const spacerBaselineAnchorRef = useRef(null); + const spacerInitialReserveRef = useRef(0); + const [bottomSpacerHeight, setBottomSpacerHeight] = useState(0); const displayLiveRuns = useMemo(() => { const deduped = new Map(); for (const run of liveRuns) { @@ -2319,10 +2509,57 @@ export function IssueChatThread({ const runtime = usePaperclipIssueRuntime({ messages, isRunning, - onSend: ({ body, reopen, reassignment }) => onAdd(body, reopen, reassignment), + onSend: ({ body, reopen, reassignment }) => { + pendingSubmitScrollRef.current = true; + return onAdd(body, reopen, reassignment); + }, onCancel: onCancelRun, }); + useEffect(() => { + const lastUserMessage = [...messages].reverse().find((m) => m.role === "user"); + const lastUserId = lastUserMessage?.id ?? null; + + if ( + pendingSubmitScrollRef.current + && lastUserId + && lastUserId !== lastUserMessageIdRef.current + ) { + pendingSubmitScrollRef.current = false; + const custom = lastUserMessage?.metadata.custom as { anchorId?: unknown } | undefined; + const anchorId = typeof custom?.anchorId === "string" ? custom.anchorId : null; + if (anchorId) { + const reserve = Math.round(window.innerHeight * SUBMIT_SCROLL_RESERVE_VH); + spacerBaselineAnchorRef.current = anchorId; + spacerInitialReserveRef.current = reserve; + setBottomSpacerHeight(reserve); + requestAnimationFrame(() => { + const el = document.getElementById(anchorId); + el?.scrollIntoView({ behavior: "smooth", block: "start" }); + }); + } + } + + lastUserMessageIdRef.current = lastUserId; + }, [messages]); + + useLayoutEffect(() => { + const anchorId = spacerBaselineAnchorRef.current; + if (!anchorId || spacerInitialReserveRef.current <= 0) return; + const userEl = document.getElementById(anchorId); + const bottomEl = bottomAnchorRef.current; + if (!userEl || !bottomEl) return; + const contentBelow = Math.max( + 0, + bottomEl.getBoundingClientRect().top - userEl.getBoundingClientRect().bottom, + ); + const next = Math.max(0, spacerInitialReserveRef.current - contentBelow); + setBottomSpacerHeight((prev) => (prev === next ? prev : next)); + if (next === 0) { + spacerBaselineAnchorRef.current = null; + spacerInitialReserveRef.current = 0; + } + }, [messages]); useLayoutEffect(() => { const composerElement = composerViewportAnchorRef.current; if (preserveComposerViewportRef.current) { @@ -2461,15 +2698,30 @@ export function IssueChatThread({ return ; }) )} + {showComposer ? ( +
+ + +
+ ) : null}
+ {showComposer ? ( +
+ ) : null}
{showComposer ? ( -
- - +
= {}): Iss currentStageType: "review", currentParticipant: { type: "agent", agentId: "agent-1", userId: null }, returnAssignee: { type: "agent", agentId: "agent-2", userId: null }, + reviewRequest: null, completedStageIds: [], lastDecisionId: null, lastDecisionOutcome: "changes_requested", diff --git a/ui/src/components/IssueProperties.tsx b/ui/src/components/IssueProperties.tsx index 5c9a38cad8..27a49bbfba 100644 --- a/ui/src/components/IssueProperties.tsx +++ b/ui/src/components/IssueProperties.tsx @@ -1,7 +1,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { pickTextColorForPillBg } from "@/lib/color-contrast"; import { Link } from "@/lib/router"; -import type { Issue, IssueLabel, IssueRelationIssueSummary, Project, WorkspaceRuntimeService } from "@paperclipai/shared"; +import type { Issue, IssueLabel, Project, WorkspaceRuntimeService } from "@paperclipai/shared"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { accessApi } from "../api/access"; import { agentsApi } from "../api/agents"; @@ -197,21 +197,6 @@ function PropertyPicker({ ); } -function IssuePillLink({ - issue, -}: { - issue: Pick | IssueRelationIssueSummary; -}) { - return ( - - {issue.identifier ?? issue.title} - - ); -} - export function IssueProperties({ issue, childIssues = [], @@ -1146,7 +1131,7 @@ export function IssueProperties({
{(issue.blockedBy ?? []).map((relation) => ( - + ))} {renderAddBlockedByButton(() => setBlockedByOpen((open) => !open))} @@ -1159,7 +1144,7 @@ export function IssueProperties({ ) : ( {(issue.blockedBy ?? []).map((relation) => ( - + ))} 0 ? (
{blockingIssues.map((relation) => ( - + ))}
) : null} @@ -1192,7 +1177,7 @@ export function IssueProperties({
{childIssues.length > 0 ? childIssues.map((child) => ( - + )) : null} {onAddSubIssue ? ( diff --git a/ui/src/components/IssueReferencePill.tsx b/ui/src/components/IssueReferencePill.tsx index 64f2c04968..78864eda16 100644 --- a/ui/src/components/IssueReferencePill.tsx +++ b/ui/src/components/IssueReferencePill.tsx @@ -1,3 +1,4 @@ +import type { ReactNode } from "react"; import type { IssueRelationIssueSummary } from "@paperclipai/shared"; import { Link } from "@/lib/router"; import { cn } from "../lib/utils"; @@ -7,11 +8,13 @@ export function IssueReferencePill({ issue, strikethrough, className, + children, }: { issue: Pick & Partial>; strikethrough?: boolean; className?: string; + children?: ReactNode; }) { const issueLabel = issue.identifier ?? issue.title; const classNames = cn( @@ -24,7 +27,7 @@ export function IssueReferencePill({ const content = ( <> {issue.status ? : null} - {issue.identifier ?? issue.title} + {children !== undefined ? children : {issue.identifier ?? issue.title}} ); diff --git a/ui/src/components/IssuesList.test.tsx b/ui/src/components/IssuesList.test.tsx index 796f80fe80..d6219038ab 100644 --- a/ui/src/components/IssuesList.test.tsx +++ b/ui/src/components/IssuesList.test.tsx @@ -22,6 +22,8 @@ const mockIssuesApi = vi.hoisted(() => ({ listLabels: vi.fn(), })); +const mockKanbanBoard = vi.hoisted(() => vi.fn()); + const mockAuthApi = vi.hoisted(() => ({ getSession: vi.fn(), })); @@ -87,7 +89,16 @@ vi.mock("./IssueRow", () => ({ })); vi.mock("./KanbanBoard", () => ({ - KanbanBoard: () => null, + KanbanBoard: (props: { issues: Issue[] }) => { + mockKanbanBoard(props); + return ( +
+ {props.issues.map((issue) => ( + {issue.title} + ))} +
+ ); + }, })); // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -189,6 +200,7 @@ describe("IssuesList", () => { container = document.createElement("div"); document.body.appendChild(container); dialogState.openNewIssue.mockReset(); + mockKanbanBoard.mockReset(); mockIssuesApi.list.mockReset(); mockIssuesApi.listLabels.mockReset(); mockAuthApi.getSession.mockReset(); @@ -404,6 +416,113 @@ describe("IssuesList", () => { }); }); + it("loads board issues with a separate result limit for each status column", async () => { + localStorage.setItem( + "paperclip:test-issues:company-1", + JSON.stringify({ viewMode: "board" }), + ); + + const parentIssue = createIssue({ + id: "issue-parent-total-limit", + title: "Parent total-limited issue", + status: "todo", + }); + const backlogIssue = createIssue({ + id: "issue-backlog", + title: "Backlog column issue", + status: "backlog", + }); + const doneIssue = createIssue({ + id: "issue-done", + title: "Done column issue", + status: "done", + }); + + mockIssuesApi.list.mockImplementation((_companyId, filters) => { + if (filters?.status === "backlog") return Promise.resolve([backlogIssue]); + if (filters?.status === "done") return Promise.resolve([doneIssue]); + return Promise.resolve([]); + }); + + const { root } = renderWithQueryClient( + undefined} + />, + container, + ); + + await waitForAssertion(() => { + expect(mockIssuesApi.list).toHaveBeenCalledWith("company-1", expect.objectContaining({ + status: "backlog", + limit: 200, + includeRoutineExecutions: true, + })); + expect(mockIssuesApi.list).toHaveBeenCalledWith("company-1", expect.objectContaining({ + status: "done", + limit: 200, + includeRoutineExecutions: true, + })); + expect(mockKanbanBoard).toHaveBeenLastCalledWith(expect.objectContaining({ + issues: expect.arrayContaining([ + expect.objectContaining({ id: "issue-backlog" }), + expect.objectContaining({ id: "issue-done" }), + ]), + })); + expect(container.textContent).toContain("Backlog column issue"); + expect(container.textContent).toContain("Done column issue"); + expect(container.textContent).not.toContain("Parent total-limited issue"); + }); + + act(() => { + root.unmount(); + }); + }); + + it("shows a refinement hint when a board column hits its server cap", async () => { + localStorage.setItem( + "paperclip:test-issues:company-1", + JSON.stringify({ viewMode: "board" }), + ); + + const cappedBacklogIssues = Array.from({ length: 200 }, (_, index) => + createIssue({ + id: `issue-backlog-${index + 1}`, + identifier: `PAP-${index + 1}`, + title: `Backlog issue ${index + 1}`, + status: "backlog", + }), + ); + + mockIssuesApi.list.mockImplementation((_companyId, filters) => { + if (filters?.status === "backlog") return Promise.resolve(cappedBacklogIssues); + return Promise.resolve([]); + }); + + const { root } = renderWithQueryClient( + undefined} + />, + container, + ); + + await waitForAssertion(() => { + expect(container.textContent).toContain("Some board columns are showing up to 200 issues. Refine filters or search to reveal the rest."); + }); + + act(() => { + root.unmount(); + }); + }); + it("caps the first paint for large issue lists", async () => { const manyIssues = Array.from({ length: 220 }, (_, index) => createIssue({ @@ -425,8 +544,8 @@ describe("IssuesList", () => { ); await waitForAssertion(() => { - expect(container.querySelectorAll('[data-testid="issue-row"]')).toHaveLength(100); - expect(container.textContent).toContain("Rendering 100 of 220 issues"); + expect(container.querySelectorAll('[data-testid="issue-row"]')).toHaveLength(150); + expect(container.textContent).toContain("Rendering 150 of 220 issues"); }); act(() => { diff --git a/ui/src/components/IssuesList.tsx b/ui/src/components/IssuesList.tsx index c2e6050179..3a63a92ab7 100644 --- a/ui/src/components/IssuesList.tsx +++ b/ui/src/components/IssuesList.tsx @@ -1,5 +1,5 @@ import { startTransition, useDeferredValue, useEffect, useMemo, useState, useCallback, useRef } from "react"; -import { useQuery } from "@tanstack/react-query"; +import { useQueries, useQuery } from "@tanstack/react-query"; import { accessApi } from "../api/access"; import { useDialog } from "../context/DialogContext"; import { useCompany } from "../context/CompanyContext"; @@ -57,13 +57,14 @@ import { KanbanBoard } from "./KanbanBoard"; import { buildIssueTree, countDescendants } from "../lib/issue-tree"; import { buildSubIssueDefaultsForViewer } from "../lib/subIssueDefaults"; import { statusBadge } from "../lib/status-colors"; -import type { Issue, Project } from "@paperclipai/shared"; +import { ISSUE_STATUSES, type Issue, type Project } from "@paperclipai/shared"; const ISSUE_SEARCH_DEBOUNCE_MS = 250; const ISSUE_SEARCH_RESULT_LIMIT = 200; const ISSUE_BOARD_COLUMN_RESULT_LIMIT = 200; -const INITIAL_ISSUE_ROW_RENDER_LIMIT = 100; +const INITIAL_ISSUE_ROW_RENDER_LIMIT = 150; const ISSUE_ROW_RENDER_BATCH_SIZE = 150; const ISSUE_ROW_RENDER_BATCH_DELAY_MS = 0; +const boardIssueStatuses = ISSUE_STATUSES; /* ── View state ── */ @@ -176,6 +177,15 @@ function sortIssues(issues: Issue[], state: IssueViewState): Issue[] { return sorted; } +function issueMatchesLocalSearch(issue: Issue, normalizedSearch: string): boolean { + if (!normalizedSearch) return true; + return [ + issue.identifier, + issue.title, + issue.description, + ].some((value) => value?.toLowerCase().includes(normalizedSearch)); +} + /* ── Component ── */ interface Agent { @@ -207,6 +217,7 @@ interface IssuesListProps { initialWorkspaces?: string[]; initialSearch?: string; searchFilters?: Omit; + searchWithinLoadedIssues?: boolean; baseCreateIssueDefaults?: Record; createIssueLabel?: string; enableRoutineVisibilityFilter?: boolean; @@ -292,6 +303,7 @@ export function IssuesList({ initialWorkspaces, initialSearch, searchFilters, + searchWithinLoadedIssues = false, baseCreateIssueDefaults, createIssueLabel, enableRoutineVisibilityFilter = false, @@ -392,9 +404,34 @@ export function IssuesList({ ...searchFilters, ...(enableRoutineVisibilityFilter ? { includeRoutineExecutions: true } : {}), }), - enabled: !!selectedCompanyId && normalizedIssueSearch.length > 0, + enabled: !!selectedCompanyId && normalizedIssueSearch.length > 0 && !searchWithinLoadedIssues, placeholderData: (previousData) => previousData, }); + const boardIssueQueries = useQueries({ + queries: boardIssueStatuses.map((status) => ({ + queryKey: [ + ...queryKeys.issues.list(selectedCompanyId ?? "__no-company__"), + "board-column", + status, + normalizedIssueSearch, + projectId ?? "__all-projects__", + searchFilters ?? {}, + ISSUE_BOARD_COLUMN_RESULT_LIMIT, + enableRoutineVisibilityFilter ? "with-routine-executions" : "without-routine-executions", + ], + queryFn: () => + issuesApi.list(selectedCompanyId!, { + ...searchFilters, + ...(normalizedIssueSearch.length > 0 ? { q: normalizedIssueSearch } : {}), + projectId, + status, + limit: ISSUE_BOARD_COLUMN_RESULT_LIMIT, + ...(enableRoutineVisibilityFilter ? { includeRoutineExecutions: true } : {}), + }), + enabled: !!selectedCompanyId && viewState.viewMode === "board" && !searchWithinLoadedIssues, + placeholderData: (previousData: Issue[] | undefined) => previousData, + })), + }); const { data: executionWorkspaces = [] } = useQuery({ queryKey: selectedCompanyId ? queryKeys.executionWorkspaces.summaryList(selectedCompanyId) @@ -571,11 +608,36 @@ export function IssuesList({ return map; }, [issues]); + const boardIssues = useMemo(() => { + if (viewState.viewMode !== "board" || searchWithinLoadedIssues) return null; + const merged = new Map(); + let isPending = false; + for (const query of boardIssueQueries) { + isPending ||= query.isPending; + for (const issue of query.data ?? []) { + merged.set(issue.id, issue); + } + } + if (merged.size > 0) return [...merged.values()]; + return isPending ? issues : []; + }, [boardIssueQueries, issues, searchWithinLoadedIssues, viewState.viewMode]); + const boardColumnLimitReached = useMemo( + () => + viewState.viewMode === "board" && + !searchWithinLoadedIssues && + boardIssueQueries.some((query) => (query.data?.length ?? 0) === ISSUE_BOARD_COLUMN_RESULT_LIMIT), + [boardIssueQueries, searchWithinLoadedIssues, viewState.viewMode], + ); + const filtered = useMemo(() => { - const sourceIssues = normalizedIssueSearch.length > 0 ? searchedIssues : issues; - const filteredByControls = applyIssueFilters(sourceIssues, viewState, currentUserId, enableRoutineVisibilityFilter); + const useRemoteSearch = normalizedIssueSearch.length > 0 && !searchWithinLoadedIssues; + const sourceIssues = boardIssues ?? (useRemoteSearch ? searchedIssues : issues); + const searchScopedIssues = normalizedIssueSearch.length > 0 && searchWithinLoadedIssues + ? sourceIssues.filter((issue) => issueMatchesLocalSearch(issue, normalizedIssueSearch)) + : sourceIssues; + const filteredByControls = applyIssueFilters(searchScopedIssues, viewState, currentUserId, enableRoutineVisibilityFilter); return sortIssues(filteredByControls, viewState); - }, [issues, searchedIssues, viewState, normalizedIssueSearch, currentUserId, enableRoutineVisibilityFilter]); + }, [boardIssues, issues, searchedIssues, searchWithinLoadedIssues, viewState, normalizedIssueSearch, currentUserId, enableRoutineVisibilityFilter]); const { data: labels } = useQuery({ queryKey: queryKeys.issues.labels(selectedCompanyId!), @@ -873,11 +935,16 @@ export function IssuesList({ {isLoading && } {error &&

{error.message}

} - {normalizedIssueSearch.length > 0 && searchedIssues.length === ISSUE_SEARCH_RESULT_LIMIT && ( + {!searchWithinLoadedIssues && normalizedIssueSearch.length > 0 && searchedIssues.length === ISSUE_SEARCH_RESULT_LIMIT && (

Showing up to {ISSUE_SEARCH_RESULT_LIMIT} matches. Refine the search to narrow further.

)} + {boardColumnLimitReached && ( +

+ Some board columns are showing up to {ISSUE_BOARD_COLUMN_RESULT_LIMIT} issues. Refine filters or search to reveal the rest. +

+ )} {!isLoading && filtered.length === 0 && viewState.viewMode === "list" && ( ({ issuesApi: mockIssuesApi, })); -function renderMarkdown(children: string, seededIssues: Array<{ identifier: string; status: string }> = []) { +function renderMarkdown(children: string, seededIssues: Array<{ identifier: string; status: string; title?: string }> = []) { const queryClient = new QueryClient({ defaultOptions: { queries: { @@ -47,6 +47,7 @@ function renderMarkdown(children: string, seededIssues: Array<{ identifier: stri id: issue.identifier, identifier: issue.identifier, status: issue.status, + title: issue.title, }); } @@ -156,9 +157,22 @@ describe("MarkdownBody", () => { expect(html).toContain('href="/issues/PAP-1271"'); expect(html).toContain("text-green-600"); expect(html).toContain(">PAP-1271<"); + expect(html).toContain('data-mention-kind="issue"'); + expect(html).toContain("paperclip-markdown-issue-ref"); expect(html).not.toContain("paperclip-mention-chip--issue"); }); + it("uses concise issue aria labels until a distinct title is available", () => { + const html = renderMarkdown("Depends on PAP-1271 and PAP-1272.", [ + { identifier: "PAP-1271", status: "done" }, + { identifier: "PAP-1272", status: "blocked", title: "Fix hover state" }, + ]); + + expect(html).toContain('aria-label="Issue PAP-1271"'); + expect(html).toContain('aria-label="Issue PAP-1272: Fix hover state"'); + expect(html).not.toContain('aria-label="Issue PAP-1271: PAP-1271"'); + }); + it("rewrites full issue URLs to internal issue links", () => { const html = renderMarkdown("See http://localhost:3100/PAP/issues/PAP-1179.", [ { identifier: "PAP-1179", status: "blocked" }, @@ -167,9 +181,33 @@ describe("MarkdownBody", () => { expect(html).toContain('href="/issues/PAP-1179"'); expect(html).toContain("text-red-600"); expect(html).toContain(">http://localhost:3100/PAP/issues/PAP-1179<"); + expect(html).toContain('data-mention-kind="issue"'); expect(html).not.toContain("paperclip-mention-chip--issue"); }); + it("linkifies plain internal issue paths in markdown text", () => { + const html = renderMarkdown("See /issues/PAP-1179 and /PAP/issues/pap-1180 for context.", [ + { identifier: "PAP-1179", status: "blocked" }, + { identifier: "PAP-1180", status: "done" }, + ]); + + expect(html).toContain('href="/issues/PAP-1179"'); + expect(html).toContain('href="/issues/PAP-1180"'); + expect(html).toContain(">/issues/PAP-1179<"); + expect(html).toContain(">/PAP/issues/pap-1180<"); + expect(html).toContain("text-red-600"); + expect(html).toContain("text-green-600"); + }); + + it("does not auto-link non-issue internal route paths", () => { + const html = renderMarkdown("Use /issues/new for the creation form, /issues/PAP-42extra as text, and /api/issues for data."); + + expect(html).toContain("Use /issues/new for the creation form, /issues/PAP-42extra as text, and /api/issues for data."); + expect(html).not.toContain('href="/issues/new"'); + expect(html).not.toContain('href="/issues/PAP-42"'); + expect(html).not.toContain('data-mention-kind="issue"'); + }); + it("rewrites issue scheme links to internal issue links", () => { const html = renderMarkdown("See issue://PAP-1310 and issue://:PAP-1311.", [ { identifier: "PAP-1310", status: "done" }, @@ -192,6 +230,22 @@ describe("MarkdownBody", () => { expect(html).toContain('href="/issues/PAP-1271"'); expect(html).toContain('PAP-1271'); expect(html).toContain("text-green-600"); + expect(html).toContain("paperclip-markdown-issue-ref"); + }); + + it("keeps trailing punctuation outside auto-linked issue references", () => { + const html = renderMarkdown("See PAP-1271: /issues/PAP-1272] and issue://PAP-1273.", [ + { identifier: "PAP-1271", status: "done" }, + { identifier: "PAP-1272", status: "blocked" }, + { identifier: "PAP-1273", status: "todo" }, + ]); + + expect(html).toContain('PAP-1271:'); + expect(html).toContain('/issues/PAP-1272]'); + expect(html).toContain('issue://PAP-1273.'); }); it("can opt out of issue reference linkification for offline previews", () => { @@ -277,7 +331,7 @@ describe("MarkdownBody", () => { expect(html).toContain('style="max-width:100%;overflow-x:auto"'); }); - it("renders internal issue links and bare identifiers as issue chips", () => { + it("renders internal issue links and bare identifiers as inline issue refs", () => { const html = renderMarkdown(`See PAP-42 and [linked task](${buildIssueReferenceHref("PAP-77")}) for follow-up.`, [ { identifier: "PAP-42", status: "done" }, { identifier: "PAP-77", status: "blocked" }, @@ -286,5 +340,7 @@ describe("MarkdownBody", () => { expect(html).toContain('href="/issues/PAP-42"'); expect(html).toContain('href="/issues/PAP-77"'); expect(html).toContain('data-mention-kind="issue"'); + expect(html).toContain("paperclip-markdown-issue-ref"); + expect(html).not.toContain("paperclip-mention-chip--issue"); }); }); diff --git a/ui/src/components/MarkdownBody.tsx b/ui/src/components/MarkdownBody.tsx index 5ab6ca149c..6ab4ff3768 100644 --- a/ui/src/components/MarkdownBody.tsx +++ b/ui/src/components/MarkdownBody.tsx @@ -4,11 +4,11 @@ import { Github } from "lucide-react"; import Markdown, { defaultUrlTransform, type Components, type Options } from "react-markdown"; import remarkGfm from "remark-gfm"; import { cn } from "../lib/utils"; +import { Link } from "@/lib/router"; import { useTheme } from "../context/ThemeContext"; import { mentionChipInlineStyle, parseMentionChipHref } from "../lib/mention-chips"; import { issuesApi } from "../api/issues"; import { queryKeys } from "../lib/queryKeys"; -import { Link } from "@/lib/router"; import { parseIssueReferenceFromHref, remarkLinkIssueReferences } from "../lib/issue-reference"; import { remarkSoftBreaks } from "../lib/remark-soft-breaks"; import { StatusIcon } from "./StatusIcon"; @@ -29,11 +29,9 @@ let mermaidLoaderPromise: Promise | null = nul function MarkdownIssueLink({ issuePathId, - href, children, }: { issuePathId: string; - href: string; children: ReactNode; }) { const { data } = useQuery({ @@ -42,14 +40,23 @@ function MarkdownIssueLink({ staleTime: 60_000, }); + const identifier = data?.identifier ?? issuePathId; + const title = data?.title ?? identifier; + const status = data?.status; + const issueLabel = title !== identifier ? `Issue ${identifier}: ${title}` : `Issue ${identifier}`; + return ( - {data ? : null} - {children} + {status ? ( + + ) : null} + {children} ); } @@ -240,7 +247,7 @@ export function MarkdownBody({ const issueRef = linkIssueReferences ? parseIssueReferenceFromHref(href) : null; if (issueRef) { return ( - + {linkChildren} ); diff --git a/ui/src/index.css b/ui/src/index.css index 95643466a5..7d42f87ed7 100644 --- a/ui/src/index.css +++ b/ui/src/index.css @@ -448,11 +448,23 @@ font-size: 0.75rem; line-height: 1.3; text-decoration: none; - vertical-align: middle; + vertical-align: baseline; white-space: nowrap; user-select: none; } +/* Strip the MDXEditor's default inline-code styling from the text inside chips + (the link label otherwise picks up a monospace font + gray tint). */ +.paperclip-mdxeditor-content a.paperclip-mention-chip, +.paperclip-mdxeditor-content a.paperclip-mention-chip code, +.paperclip-mdxeditor-content a.paperclip-project-mention-chip, +.paperclip-mdxeditor-content a.paperclip-project-mention-chip code { + font-family: inherit; + background: none; + color: inherit; + padding: 0; +} + .paperclip-mdxeditor-content a.paperclip-mention-chip::before, a.paperclip-mention-chip::before { content: ""; @@ -768,6 +780,13 @@ a.paperclip-mention-chip[data-mention-kind="agent"]::before { background: color-mix(in oklab, var(--accent) 42%, transparent); } +/* Inline issue references in markdown: no pill chrome, just a status icon + beside the link label — keeps the pair from splitting across lines. */ +.paperclip-markdown-issue-ref { + display: inline; + white-space: nowrap; +} + .dark .paperclip-markdown a { color: color-mix(in oklab, var(--foreground) 80%, #58a6ff 20%); } @@ -832,9 +851,11 @@ a.paperclip-mention-chip[data-mention-kind="agent"]::before { background: transparent; } -/* Project mention chips rendered inside MarkdownBody */ +/* Mention chips rendered inline in prose (MarkdownBody or inline anchors) */ a.paperclip-mention-chip, -a.paperclip-project-mention-chip { +a.paperclip-project-mention-chip, +span.paperclip-mention-chip, +span.paperclip-project-mention-chip { display: inline-flex; align-items: center; gap: 0.25rem; @@ -845,10 +866,25 @@ a.paperclip-project-mention-chip { font-size: 0.75rem; line-height: 1.3; text-decoration: none; - vertical-align: middle; + /* Align the pill relative to the surrounding text baseline instead of its + x-height midpoint so it sits on the text line rather than floating above. */ + vertical-align: baseline; white-space: nowrap; } +/* When the identifier inside a chip is backtick-wrapped in markdown, strip the + inline-code monospace/gray styling so the pill label reads cleanly. */ +.paperclip-markdown a.paperclip-mention-chip code, +.paperclip-markdown a.paperclip-project-mention-chip code, +.paperclip-markdown span.paperclip-mention-chip code, +.paperclip-markdown span.paperclip-project-mention-chip code { + font-family: inherit; + background: none; + color: inherit; + padding: 0; + font-size: inherit; +} + /* Keep MDXEditor popups above app dialogs, even when they portal to . */ [class*="_popupContainer_"] { z-index: 81 !important; diff --git a/ui/src/lib/issue-reference.test.ts b/ui/src/lib/issue-reference.test.ts index 8eeca53a19..cd0f038a7e 100644 --- a/ui/src/lib/issue-reference.test.ts +++ b/ui/src/lib/issue-reference.test.ts @@ -4,6 +4,7 @@ import { parseIssuePathIdFromPath, parseIssueReferenceFromHref } from "./issue-r describe("issue-reference", () => { it("extracts issue ids from company-scoped issue paths", () => { expect(parseIssuePathIdFromPath("/PAP/issues/PAP-1271")).toBe("PAP-1271"); + expect(parseIssuePathIdFromPath("/PAP/issues/pap-1272")).toBe("PAP-1272"); expect(parseIssuePathIdFromPath("/issues/PAP-1179")).toBe("PAP-1179"); expect(parseIssuePathIdFromPath("/issues/:id")).toBeNull(); }); @@ -32,6 +33,10 @@ describe("issue-reference", () => { issuePathId: "PAP-1179", href: "/issues/PAP-1179", }); + expect(parseIssueReferenceFromHref("/PAP/issues/pap-1180")).toEqual({ + issuePathId: "PAP-1180", + href: "/issues/PAP-1180", + }); expect(parseIssueReferenceFromHref("issue://PAP-1310")).toEqual({ issuePathId: "PAP-1310", href: "/issues/PAP-1310", diff --git a/ui/src/lib/issue-reference.ts b/ui/src/lib/issue-reference.ts index 5e03e10eb2..cbef86ffd8 100644 --- a/ui/src/lib/issue-reference.ts +++ b/ui/src/lib/issue-reference.ts @@ -7,7 +7,7 @@ type MarkdownNode = { const BARE_ISSUE_IDENTIFIER_RE = /^[A-Z][A-Z0-9]+-\d+$/i; const ISSUE_SCHEME_RE = /^issue:\/\/:?([^?#\s]+)(?:[?#].*)?$/i; -const ISSUE_REFERENCE_TOKEN_RE = /issue:\/\/:?[^\s<>()]+|https?:\/\/[^\s<>()]+|\b[A-Z][A-Z0-9]+-\d+\b/gi; +const ISSUE_REFERENCE_TOKEN_RE = /issue:\/\/:?[^\s<>()]+|https?:\/\/[^\s<>()]+|\/(?:[^\s<>()/]+\/)*issues\/[A-Z][A-Z0-9]+-\d+(?=$|[\s<>)\],.;!?:])|\b[A-Z][A-Z0-9]+-\d+\b/gi; export function parseIssuePathIdFromPath(pathOrUrl: string | null | undefined): string | null { if (!pathOrUrl) return null; @@ -29,7 +29,7 @@ export function parseIssuePathIdFromPath(pathOrUrl: string | null | undefined): if (issueIndex === -1 || issueIndex === segments.length - 1) return null; const issuePathId = decodeURIComponent(segments[issueIndex + 1] ?? ""); if (!issuePathId || issuePathId.startsWith(":")) return null; - return issuePathId; + return BARE_ISSUE_IDENTIFIER_RE.test(issuePathId) ? issuePathId.toUpperCase() : issuePathId; } export function parseIssueReferenceFromHref(href: string | null | undefined) { @@ -66,12 +66,17 @@ function splitTrailingPunctuation(token: string) { while (core.length > 0) { const lastChar = core.at(-1); - if (!lastChar || !/[),.;!?]/.test(lastChar)) break; + if (!lastChar || !/[),.;!?:\]]/.test(lastChar)) break; if (lastChar === ")") { const openCount = (core.match(/\(/g) ?? []).length; const closeCount = (core.match(/\)/g) ?? []).length; if (closeCount <= openCount) break; } + if (lastChar === "]") { + const openCount = (core.match(/\[/g) ?? []).length; + const closeCount = (core.match(/\]/g) ?? []).length; + if (closeCount <= openCount) break; + } trailing = `${lastChar}${trailing}`; core = core.slice(0, -1); } diff --git a/ui/src/lib/issue-tree.test.ts b/ui/src/lib/issue-tree.test.ts index 75d38f874a..955f3a0d98 100644 --- a/ui/src/lib/issue-tree.test.ts +++ b/ui/src/lib/issue-tree.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import type { Issue } from "@paperclipai/shared"; -import { buildIssueTree, countDescendants } from "./issue-tree"; +import { buildIssueTree, countDescendants, filterIssueDescendants } from "./issue-tree"; function makeIssue(id: string, parentId: string | null = null): Issue { return { @@ -128,3 +128,33 @@ describe("countDescendants", () => { expect(countDescendants("nonexistent", childMap)).toBe(0); }); }); + +describe("filterIssueDescendants", () => { + it("returns only children and deeper descendants of the requested root", () => { + const root = makeIssue("root"); + const child = makeIssue("child", "root"); + const grandchild = makeIssue("grandchild", "child"); + const unrelatedParent = makeIssue("other"); + const unrelatedChild = makeIssue("other-child", "other"); + + expect(filterIssueDescendants("root", [ + root, + child, + grandchild, + unrelatedParent, + unrelatedChild, + ]).map((issue) => issue.id)).toEqual(["child", "grandchild"]); + }); + + it("handles stale broad issue-list responses without requiring the root in the list", () => { + const child = makeIssue("child", "root"); + const grandchild = makeIssue("grandchild", "child"); + const globalIssue = makeIssue("global"); + + expect(filterIssueDescendants("root", [ + globalIssue, + child, + grandchild, + ]).map((issue) => issue.id)).toEqual(["child", "grandchild"]); + }); +}); diff --git a/ui/src/lib/issue-tree.ts b/ui/src/lib/issue-tree.ts index e88709ed12..2570705314 100644 --- a/ui/src/lib/issue-tree.ts +++ b/ui/src/lib/issue-tree.ts @@ -34,3 +34,39 @@ export function countDescendants(id: string, childMap: Map): nu const children = childMap.get(id) ?? []; return children.reduce((sum, c) => sum + 1 + countDescendants(c.id, childMap), 0); } + +/** + * Filters a flat issue list to only descendants of `rootId`. + * + * This is intentionally useful even when the list contains unrelated issues: + * stale servers may ignore newer descendant-scoped query params, and the UI + * must still avoid rendering global issue data in a sub-issue panel. + */ +export function filterIssueDescendants(rootId: string, items: Issue[]): Issue[] { + const childrenByParentId = new Map(); + for (const item of items) { + if (!item.parentId) continue; + const siblings = childrenByParentId.get(item.parentId) ?? []; + siblings.push(item); + childrenByParentId.set(item.parentId, siblings); + } + + const descendants: Issue[] = []; + const seen = new Set([rootId]); + let frontier = [rootId]; + + while (frontier.length > 0) { + const nextFrontier: string[] = []; + for (const parentId of frontier) { + for (const child of childrenByParentId.get(parentId) ?? []) { + if (seen.has(child.id)) continue; + seen.add(child.id); + descendants.push(child); + nextFrontier.push(child.id); + } + } + frontier = nextFrontier; + } + + return descendants; +} diff --git a/ui/src/lib/queryKeys.ts b/ui/src/lib/queryKeys.ts index c345b87afc..c61b216a2f 100644 --- a/ui/src/lib/queryKeys.ts +++ b/ui/src/lib/queryKeys.ts @@ -41,6 +41,8 @@ export const queryKeys = { ["issues", companyId, "project", projectId] as const, listByParent: (companyId: string, parentId: string) => ["issues", companyId, "parent", parentId] as const, + listByDescendantRoot: (companyId: string, rootIssueId: string) => + ["issues", companyId, "descendants", rootIssueId] as const, listByExecutionWorkspace: (companyId: string, executionWorkspaceId: string) => ["issues", companyId, "execution-workspace", executionWorkspaceId] as const, detail: (id: string) => ["issues", "detail", id] as const, diff --git a/ui/src/pages/IssueDetail.test.tsx b/ui/src/pages/IssueDetail.test.tsx index 7fbd28e217..bd47590b30 100644 --- a/ui/src/pages/IssueDetail.test.tsx +++ b/ui/src/pages/IssueDetail.test.tsx @@ -850,8 +850,8 @@ describe("IssueDetail", () => { }; mockIssuesApi.get.mockResolvedValue(createIssue()); - mockIssuesApi.list.mockImplementation((_companyId, filters?: { parentId?: string }) => - Promise.resolve(filters?.parentId === "issue-1" ? [childIssue] : []), + mockIssuesApi.list.mockImplementation((_companyId, filters?: { descendantOf?: string }) => + Promise.resolve(filters?.descendantOf === "issue-1" ? [childIssue] : []), ); mockIssuesApi.getTreeControlState.mockImplementation(() => Promise.resolve({ activePauseHold: activePauseHoldState }), @@ -937,8 +937,8 @@ describe("IssueDetail", () => { }); mockIssuesApi.get.mockResolvedValue(createIssue()); - mockIssuesApi.list.mockImplementation((_companyId, filters?: { parentId?: string }) => - Promise.resolve(filters?.parentId === "issue-1" ? [childIssue] : []), + mockIssuesApi.list.mockImplementation((_companyId, filters?: { descendantOf?: string }) => + Promise.resolve(filters?.descendantOf === "issue-1" ? [childIssue] : []), ); mockIssuesApi.previewTreeControl.mockResolvedValue(pausePreview); mockIssuesApi.createTreeHold.mockResolvedValue({ hold: pauseHold, preview: pausePreview }); @@ -1031,8 +1031,8 @@ describe("IssueDetail", () => { }); mockIssuesApi.get.mockResolvedValue(createIssue()); - mockIssuesApi.list.mockImplementation((_companyId, filters?: { parentId?: string }) => - Promise.resolve(filters?.parentId === "issue-1" ? [childIssue] : []), + mockIssuesApi.list.mockImplementation((_companyId, filters?: { descendantOf?: string }) => + Promise.resolve(filters?.descendantOf === "issue-1" ? [childIssue] : []), ); mockIssuesApi.listTreeHolds.mockImplementation((_issueId, filters?: { mode?: string }) => Promise.resolve(filters?.mode === "cancel" ? [cancelHold] : []), @@ -1106,8 +1106,8 @@ describe("IssueDetail", () => { }); mockIssuesApi.get.mockResolvedValue(createIssue()); - mockIssuesApi.list.mockImplementation((_companyId, filters?: { parentId?: string }) => - Promise.resolve(filters?.parentId === "issue-1" ? [childIssue] : []), + mockIssuesApi.list.mockImplementation((_companyId, filters?: { descendantOf?: string }) => + Promise.resolve(filters?.descendantOf === "issue-1" ? [childIssue] : []), ); mockIssuesApi.previewTreeControl.mockResolvedValue(createCancelPreview(24)); mockAuthApi.getSession.mockResolvedValue({ diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index a6e1830034..a4345e0a61 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -98,6 +98,7 @@ import { Textarea } from "@/components/ui/textarea"; import { formatIssueActivityAction } from "@/lib/activity-format"; import { buildIssuePropertiesPanelKey } from "../lib/issue-properties-panel-key"; import { shouldRenderRichSubIssuesSection } from "../lib/issue-detail-subissues"; +import { filterIssueDescendants } from "../lib/issue-tree"; import { buildSubIssueDefaultsForViewer } from "../lib/subIssueDefaults"; import { Activity as ActivityIcon, @@ -1132,9 +1133,9 @@ export function IssueDetail() { const { data: rawChildIssues = [], isLoading: childIssuesLoading } = useQuery({ queryKey: issue?.id && resolvedCompanyId - ? queryKeys.issues.listByParent(resolvedCompanyId, issue.id) + ? queryKeys.issues.listByDescendantRoot(resolvedCompanyId, issue.id) : ["issues", "parent", "pending"], - queryFn: () => issuesApi.list(resolvedCompanyId!, { parentId: issue!.id }), + queryFn: () => issuesApi.list(resolvedCompanyId!, { descendantOf: issue!.id }), enabled: !!resolvedCompanyId && !!issue?.id, placeholderData: keepPreviousDataForSameQueryTail(issue?.id ?? "pending"), }); @@ -1286,8 +1287,11 @@ export function IssueDetail() { [issue?.project, issue?.projectId, orderedProjects], ); const childIssues = useMemo( - () => [...rawChildIssues].sort((a, b) => new Date(a.createdAt).getTime() - new Date(b.createdAt).getTime()), - [rawChildIssues], + () => { + const descendants = issue?.id ? filterIssueDescendants(issue.id, rawChildIssues) : rawChildIssues; + return [...descendants].sort((a, b) => new Date(a.createdAt).getTime() - new Date(b.createdAt).getTime()); + }, + [issue?.id, rawChildIssues], ); const liveIssueIds = useMemo(() => collectLiveIssueIds(companyLiveRuns), [companyLiveRuns]); const issuePanelKey = useMemo( @@ -3133,7 +3137,8 @@ export function IssueDetail() { projectId={issue.projectId ?? undefined} viewStateKey={`paperclip:issue-detail:${issue.id}:subissues-view`} issueLinkState={resolvedIssueDetailState ?? location.state} - searchFilters={{ parentId: issue.id }} + searchFilters={{ descendantOf: issue.id }} + searchWithinLoadedIssues baseCreateIssueDefaults={buildSubIssueDefaultsForViewer(issue, currentUserId)} createIssueLabel="Sub-issue" onUpdateIssue={handleChildIssueUpdate}