mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-27 19:35:13 +01:00
## Thinking Path > - Paperclip is the control plane people use to create, configure, and run AI agents for work. > - This change sits in the server-side agent lifecycle and secret-binding subsystem, where adapter config `env` entries can reference company secrets. > - An incident (while trying to configure a Novita sandbox) showed that an agent can reach a broken runtime state if `adapterConfig.env` contains `secret_ref` entries but the matching `company_secret_bindings` rows are missing. > - The immediate run-path guard and error-surfacing work made the failure diagnosable, but they did not fully prevent new broken agents from being created. > - The risk came from create and approval flows being responsible for remembering to sync bindings at each call site, which is easy to miss as new flows are added. > - This pull request moves the invariant into `agentService` create/update/activate paths, keeps the existing hire-flow fix, and adds regression coverage for create, update, and legacy pending-approval recovery. > - The benefit is that agent secret binding integrity is enforced closer to the data mutation point, so future callers inherit the protection automatically. ## Linked Issues or Issue Description Refs #8309 ### What happened? A Paperclip agent could persist `adapterConfig.env` `secret_ref` entries without matching agent-scoped `company_secret_bindings` rows. When that happened, the config UI could still look configured, but the real run path failed pre-dispatch because the secret was not actually bound to that agent. ### Expected behavior Every normal agent create, config-update, and pending-approval activation flow should leave the agent with secret bindings that match its persisted secret-ref env config. ### Steps to reproduce 1. Create or activate an agent through a flow that persists `adapterConfig.env` secret refs without synchronizing `company_secret_bindings`. 2. Observe that the config state can still appear populated. 3. Start a run for that agent. 4. Observe that pre-dispatch binding validation fails because the secret reference exists but the agent binding does not. ### Deployment mode Local dev (`pnpm dev`) ### Installation method Built from source (`pnpm dev` / `pnpm build`) ### Agent adapter(s) involved - Claude Code - Not adapter-specific (core bug) ### Database mode Embedded PGlite / embedded local dev database flow ### Access context Board (human operator) created or approved the agent; agent runtime later consumed the config. ### Additional context This PR focuses on preventing new broken states from normal service flows and on backfilling the covered legacy pending-approval activation path. ## What Changed - Kept the existing branch-local hire-flow fix that synchronized bindings for route and approval paths. - Moved the binding integrity invariant into `agentService.create()`, `agentService.update()` when `adapterConfig` changes, and `agentService.activatePendingApproval()`. - Added `server/src/__tests__/agents-service-secret-bindings.test.ts` covering create-time sync, update-time resync, and backfill for legacy pending-approval agents. - Removed now-redundant route-layer and approval-layer binding sync calls once the service layer became authoritative. - Simplified the affected unit tests so route/approval tests no longer assert service-owned binding writes directly. ## Verification - `pnpm --filter @paperclipai/server typecheck` - `pnpm exec vitest run server/src/__tests__/agents-service-secret-bindings.test.ts server/src/__tests__/approvals-service.test.ts server/src/__tests__/agent-skills-routes.test.ts` ## Risks - Low to medium risk. - This changes where secret-binding synchronization is enforced, so any unexpected caller that relied on upper-layer manual sync behavior could behave differently. - Agent create/update/activation flows now perform binding synchronization consistently, which adds binding-table writes at those mutation points. - This PR does not retroactively scan and heal every already-broken historical agent row; it prevents and backfills through the covered service flows. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex / GPT-5 Codex class model via `codex_local` - Session model family: GPT-5 Codex - Tool-assisted coding with shell, git, HTTP, and local test execution - Reasoning mode: medium interactive tool-use workflow ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
113 lines
4.1 KiB
TypeScript
113 lines
4.1 KiB
TypeScript
import { useQuery } from "@tanstack/react-query";
|
|
import { Link } from "@/lib/router";
|
|
import { AGENT_ROLE_LABELS, type Agent, type AgentRuntimeState } from "@paperclipai/shared";
|
|
import { agentsApi } from "../api/agents";
|
|
import { useCompany } from "../context/CompanyContext";
|
|
import { getAdapterLabel } from "../adapters/adapter-display-registry";
|
|
import { queryKeys } from "../lib/queryKeys";
|
|
import { AgentStatusBadge } from "./StatusBadge";
|
|
import { Identity } from "./Identity";
|
|
import { formatDate, agentUrl } from "../lib/utils";
|
|
import { Separator } from "@/components/ui/separator";
|
|
|
|
interface AgentPropertiesProps {
|
|
agent: Agent;
|
|
runtimeState?: AgentRuntimeState;
|
|
}
|
|
|
|
const roleLabels = AGENT_ROLE_LABELS as Record<string, string>;
|
|
|
|
function PropertyRow({ label, children }: { label: string; children: React.ReactNode }) {
|
|
return (
|
|
<div className="flex items-start gap-3 py-1.5">
|
|
<span className="text-xs text-muted-foreground shrink-0 w-20 mt-0.5">{label}</span>
|
|
<div className="flex items-center gap-1.5 min-w-0 flex-1 flex-wrap">{children}</div>
|
|
</div>
|
|
);
|
|
}
|
|
|
|
export function AgentProperties({ agent, runtimeState }: AgentPropertiesProps) {
|
|
const { selectedCompanyId } = useCompany();
|
|
const lastErrorIsActive = agent.status === "error";
|
|
|
|
const { data: agents } = useQuery({
|
|
queryKey: queryKeys.agents.list(selectedCompanyId!),
|
|
queryFn: () => agentsApi.list(selectedCompanyId!),
|
|
enabled: !!selectedCompanyId && !!agent.reportsTo,
|
|
});
|
|
|
|
const reportsToAgent = agent.reportsTo ? agents?.find((a) => a.id === agent.reportsTo) : null;
|
|
|
|
return (
|
|
<div className="space-y-4">
|
|
<div className="space-y-1">
|
|
<PropertyRow label="Status">
|
|
<AgentStatusBadge status={agent.status} />
|
|
</PropertyRow>
|
|
{lastErrorIsActive && agent.errorReason && (
|
|
<PropertyRow label="Error reason">
|
|
<span className="text-xs text-red-600 dark:text-red-400 break-words min-w-0">
|
|
{agent.errorReason}
|
|
</span>
|
|
</PropertyRow>
|
|
)}
|
|
<PropertyRow label="Role">
|
|
<span className="text-sm">{roleLabels[agent.role] ?? agent.role}</span>
|
|
</PropertyRow>
|
|
{agent.title && (
|
|
<PropertyRow label="Title">
|
|
<span className="text-sm">{agent.title}</span>
|
|
</PropertyRow>
|
|
)}
|
|
<PropertyRow label="Adapter">
|
|
<span className="text-sm font-mono">{getAdapterLabel(agent.adapterType)}</span>
|
|
</PropertyRow>
|
|
</div>
|
|
|
|
<Separator />
|
|
|
|
<div className="space-y-1">
|
|
{(runtimeState?.sessionDisplayId ?? runtimeState?.sessionId) && (
|
|
<PropertyRow label="Session">
|
|
<span className="text-xs font-mono">
|
|
{String(runtimeState.sessionDisplayId ?? runtimeState.sessionId).slice(0, 12)}...
|
|
</span>
|
|
</PropertyRow>
|
|
)}
|
|
{runtimeState?.lastError && (
|
|
<PropertyRow label={lastErrorIsActive ? "Last error" : "Last run error"}>
|
|
<span
|
|
className={
|
|
lastErrorIsActive
|
|
? "text-xs text-red-600 dark:text-red-400 break-words min-w-0"
|
|
: "text-xs text-muted-foreground break-words min-w-0"
|
|
}
|
|
>
|
|
{runtimeState.lastError}
|
|
</span>
|
|
</PropertyRow>
|
|
)}
|
|
{agent.lastHeartbeatAt && (
|
|
<PropertyRow label="Last Heartbeat">
|
|
<span className="text-sm">{formatDate(agent.lastHeartbeatAt)}</span>
|
|
</PropertyRow>
|
|
)}
|
|
{agent.reportsTo && (
|
|
<PropertyRow label="Reports To">
|
|
{reportsToAgent ? (
|
|
<Link to={agentUrl(reportsToAgent)} className="hover:underline">
|
|
<Identity name={reportsToAgent.name} size="sm" />
|
|
</Link>
|
|
) : (
|
|
<span className="text-sm font-mono">{agent.reportsTo.slice(0, 8)}</span>
|
|
)}
|
|
</PropertyRow>
|
|
)}
|
|
<PropertyRow label="Created">
|
|
<span className="text-sm">{formatDate(agent.createdAt)}</span>
|
|
</PropertyRow>
|
|
</div>
|
|
</div>
|
|
);
|
|
}
|