Conversation
|
@pullfrog update issue description based on changes in this PR |
|
Updated the PR description with a comprehensive summary of all changes in this PR, covering:
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Well-structured feature with solid test coverage and clean separation between same-project duplication and cross-project import. The portability layer, dependency reconciliation, and credential-missing warnings are thoughtfully designed.
Two actionable issues: a duplicate sourceProjectId === projectId check that runs in both the handler and the data-access layer (pick one), and buildWarningSummary is copy-pasted across two UI components. The unrelated layout/header refactor and timeline cleanup would ideally be in their own PR to keep the diff focused.
Claude Opus ｜ 𝕏
| message: | ||
| 'Source and target project must differ. Use /duplicate to copy within the same project.', | ||
| }); | ||
| } |
There was a problem hiding this comment.
This sourceProjectId === projectId guard is duplicated — importFullAgentServerSide in agentImport.ts:639 performs the same check and throws the same error. Having it in both places means a future wording change could diverge. Remove one; keeping it in the DAL is the safer choice since DAL validation can't be bypassed.
|
|
||
| if (process.env.ENVIRONMENT !== 'test') { | ||
| const userId = c.get('userId'); | ||
| const tenantId = c.get('tenantId'); |
There was a problem hiding this comment.
The inner const tenantId = c.get('tenantId') shadows the outer tenantId from c.req.valid('param'). This works because they should always be the same value, but it makes the code harder to reason about. Consider renaming or removing the outer destructure from the return object and using the middleware-provided one consistently.
| function buildWarningSummary(warnings: ImportAgentWarning[]) { | ||
| const disconnectedTools = warnings | ||
| .filter((warning) => warning.resourceType === 'tool') | ||
| .map((warning) => warning.resourceId); | ||
| const disconnectedExternalAgents = warnings | ||
| .filter((warning) => warning.resourceType === 'externalAgent') | ||
| .map((warning) => warning.resourceId); | ||
|
|
||
| const parts = []; | ||
|
|
||
| if (disconnectedTools.length) { | ||
| parts.push(`tools: ${disconnectedTools.join(', ')}`); | ||
| } | ||
|
|
||
| if (disconnectedExternalAgents.length) { | ||
| parts.push(`external agents: ${disconnectedExternalAgents.join(', ')}`); | ||
| } | ||
|
|
||
| return parts.join(' | '); | ||
| } |
There was a problem hiding this comment.
buildWarningSummary is duplicated verbatim in import-agent-section.tsx. Extract it to a shared utility (e.g., lib/utils/agent-warnings.ts) to avoid drift.
| await createSkill(params.targetDb)({ | ||
| tenantId: params.targetScopes.tenantId, | ||
| projectId: params.targetScopes.projectId, | ||
| name: params.sourceSkill.name, | ||
| description: params.sourceSkill.description, | ||
| content: params.sourceSkill.content, | ||
| metadata: params.sourceSkill.metadata ?? null, | ||
| files: normalizeSkillFiles(params.sourceSkill.files), | ||
| }); |
There was a problem hiding this comment.
This relies on the invariant that skill.id === skill.name (enforced by createSkill at skills.ts:416). If that invariant ever changes, the imported skill would get a different ID than what the agent references. Consider passing the source id explicitly or adding a comment documenting the assumption.
| const tools = new Map<string, ToolSelect>(); | ||
| for (const toolId of referencedDependencyIds.toolIds) { | ||
| const sourceTool = await getToolById(params.sourceDb)({ | ||
| scopes: params.sourceScopes, | ||
| toolId, | ||
| }); | ||
|
|
||
| if (!sourceTool) { | ||
| throw createApiError({ | ||
| code: 'not_found', | ||
| message: `Source tool '${toolId}' not found`, | ||
| }); | ||
| } | ||
|
|
||
| tools.set(toolId, sourceTool); | ||
| } | ||
|
|
||
| const externalAgents = new Map<string, ExternalAgentSelect>(); | ||
| for (const externalAgentId of referencedDependencyIds.externalAgentIds) { | ||
| const sourceExternalAgent = await getExternalAgent(params.sourceDb)({ | ||
| scopes: params.sourceScopes, | ||
| externalAgentId, | ||
| }); | ||
|
|
||
| if (!sourceExternalAgent) { | ||
| throw createApiError({ | ||
| code: 'not_found', | ||
| message: `Source external agent '${externalAgentId}' not found`, | ||
| }); | ||
| } | ||
|
|
||
| externalAgents.set(externalAgentId, sourceExternalAgent); | ||
| } | ||
|
|
||
| const dataComponents = new Map<string, DataComponentSelect>(); | ||
| for (const dataComponentId of referencedDependencyIds.dataComponentIds) { | ||
| const sourceDataComponent = await getDataComponent(params.sourceDb)({ | ||
| scopes: params.sourceScopes, | ||
| dataComponentId, | ||
| }); | ||
|
|
||
| if (!sourceDataComponent) { | ||
| throw createApiError({ | ||
| code: 'not_found', | ||
| message: `Source data component '${dataComponentId}' not found`, | ||
| }); | ||
| } | ||
|
|
||
| dataComponents.set(dataComponentId, sourceDataComponent); | ||
| } | ||
|
|
||
| const artifactComponents = new Map<string, ArtifactComponentSelect>(); | ||
| for (const artifactComponentId of referencedDependencyIds.artifactComponentIds) { | ||
| const sourceArtifactComponent = await getArtifactComponentById(params.sourceDb)({ | ||
| scopes: params.sourceScopes, | ||
| id: artifactComponentId, | ||
| }); | ||
|
|
||
| if (!sourceArtifactComponent) { | ||
| throw createApiError({ | ||
| code: 'not_found', | ||
| message: `Source artifact component '${artifactComponentId}' not found`, | ||
| }); | ||
| } | ||
|
|
||
| artifactComponents.set(artifactComponentId, sourceArtifactComponent); | ||
| } | ||
|
|
||
| const functions = new Map<string, FunctionApiInsert>(); | ||
| for (const functionId of referencedDependencyIds.functionIds) { | ||
| const sourceFunction = await getFunction(params.sourceDb)({ | ||
| scopes: params.sourceScopes, | ||
| functionId, | ||
| }); | ||
|
|
||
| if (!sourceFunction) { | ||
| throw createApiError({ | ||
| code: 'not_found', | ||
| message: `Source function '${functionId}' not found`, | ||
| }); | ||
| } | ||
|
|
||
| functions.set(functionId, sourceFunction); | ||
| } | ||
|
|
||
| const skills = new Map<string, SourceSkill>(); | ||
| for (const skillId of referencedDependencyIds.skillIds) { | ||
| const sourceSkill = await getSkillByIdWithFiles(params.sourceDb)({ | ||
| scopes: params.sourceScopes, | ||
| skillId, | ||
| }); | ||
|
|
||
| if (!sourceSkill) { | ||
| throw createApiError({ | ||
| code: 'not_found', | ||
| message: `Source skill '${skillId}' not found`, | ||
| }); | ||
| } | ||
|
|
||
| skills.set(skillId, sourceSkill); | ||
| } | ||
|
|
||
| return { | ||
| ...referencedDependencyIds, | ||
| tools, | ||
| externalAgents, | ||
| dataComponents, | ||
| artifactComponents, | ||
| functions, | ||
| skills, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
All dependency loads are sequential (for ... await). For large agents referencing many tools/external-agents/components, this could become a latency bottleneck. Consider parallelizing the loads per resource type using Promise.all (the same pattern used in the tests). Not a blocker, but worth noting for future optimization.
| <div className="ml-auto flex items-center gap-1"> | ||
| {[ | ||
| { href: getMailUrl(tenantId), title: 'Help' }, | ||
| { href: DOCS_BASE_URL, title: 'Docs' }, | ||
| ].map(({ href, title }) => ( | ||
| <a | ||
| key={title} | ||
| href={href} | ||
| className="text-sm text-muted-foreground hover:text-foreground focus-visible:text-foreground focus-visible:outline-none px-2 py-1 rounded-sm" | ||
| {...(href.startsWith('https://') && { | ||
| target: '_blank', | ||
| rel: 'noopener noreferrer', | ||
| })} | ||
| > | ||
| {title} | ||
| </a> | ||
| ))} | ||
| {separator} | ||
| <HeaderMenus /> | ||
| </div> |
There was a problem hiding this comment.
The help/docs links refactor and HeaderMenus slimming are unrelated to agent duplication/import. Mixing unrelated UI refactors into a feature PR makes the diff harder to review and bisect. Consider splitting these into a separate PR.
| const [collapsedAiMessages, setCollapsedAiMessages] = useState(new Set<string>()); | ||
| const [aiMessagesGloballyCollapsed, setAiMessagesGloballyCollapsed] = useState(enableAutoScroll); | ||
|
|
||
| // State for collapsible tree nodes (lifted from HierarchicalTimeline) | ||
| const [collapsedNodes, setCollapsedNodes] = useState<Set<string>>(new Set()); | ||
| const [collapsedNodes, setCollapsedNodes] = useState(new Set<string>()); |
There was a problem hiding this comment.
Removing redundant useState type parameters is fine, but this is an unrelated cleanup. Same comment as the layout refactor — keeping the diff focused to the feature helps reviewability.
|
TL;DR — Adds two new agent copy flows: same-project duplication ( Key changes
Summary ｜ 28 files ｜ 48 commits ｜ base: Cross-project import with dependency resolution
The import follows a strict conflict-resolution strategy for each dependency type: if the resource doesn't exist in the target, create it; if it exists with identical configuration, reuse it; if it differs, return a
API route registration and authorization
The source-project check returns
Unified copy UI in the dashboard
The
Test coverage
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review Summary
(8) Total Issues | Risk: Medium
🟠⚠️ Major (3) 🟠⚠️
🟠 1) agents-api/src/domains/manage/routes/agent.ts:83 API key users bypass source project authorization
Issue: When a user authenticates with an API key (userId.startsWith('apikey:')), the canViewProject check for the source project is completely skipped. While API keys are scoped to a single tenant via middleware, they bypass project-level permissions entirely.
Why: An API key with access to Project A could import agents from Project B within the same tenant without explicit view access to Project B. This violates least-privilege principles. API keys should only access projects they're explicitly granted access to.
Fix: Remove the !userId.startsWith('apikey:') condition from line 83, or add an explicit project-level authorization check for API keys. If API keys intentionally have full project access within their tenant, document this behavior clearly.
Refs:
- agent.ts:83 — bypass condition
- permissions.ts —
canUseProjectStrictshows system/apikey bypass is intentional
🟠 2) packages/agents-core/src/data-access/manage/agentDuplicate.ts:44-54 TOCTOU race condition in agent existence check
Issue: The function checks if the target agent exists (line 45-48), then creates the agent (line 63-66) without a transaction. Between the check and create, a concurrent request could create an agent with the same ID.
Why: While the catch block handles unique constraint errors, this is inconsistent with agentImport.ts which properly uses a transaction. The race window means the first request's check passes but its create fails with an unclear error path.
Fix: Wrap the existence check and creation in a transaction, similar to importFullAgentServerSide:
return await db.transaction(async (tx) => {
const targetAgent = await getAgentById(tx)({ scopes: {...} });
if (targetAgent) { throw conflict... }
return await createFullAgentServerSide(tx, logger)(targetScopes, duplicateAgentDefinition);
});Refs:
- agentImport.ts:597 — correct transaction pattern
🟠 3) agentImport.ts + agentDuplicate.test.ts Missing test coverage for race conditions and dependency conflicts
Issue: Multiple test coverage gaps exist:
- No test for unique constraint race condition in
duplicateFullAgentServerSide - No conflict tests for external agents, data components, artifact components, functions, or skills (only tools tested)
- No test for missing source dependency error paths
Why: If conflict detection logic is broken for non-tool dependencies, users importing agents with conflicting dependencies won't get proper 409 errors. The race condition path could return incorrect error responses in production.
Fix: Add tests for:
- Race condition: mock
getAgentByIdto return null, havecreateFullAgentServerSidethrow unique constraint error - Each dependency type conflict: create source dependency, create conflicting version in target, verify 409
Refs:
- agentDuplicate.test.ts:310-340 — existing 409 test only covers pre-check path
- agentImport.test.ts:618-678 — tool conflict test to replicate for other types
Inline Comments:
- 🟠 Major:
agent.ts:74Variable shadowing in tenantId (see inline)
🟡 Minor (3) 🟡
🟡 1) packages/agents-core/src/data-access/manage/agentImport.ts:1 HTTPException import breaks data-access layer boundary
Issue: The data-access modules consistently use createApiError from utils/error.ts, but agentImport.ts imports HTTPException directly from hono/http-exception and uses instanceof HTTPException check (line 712).
Why: Introduces Hono framework dependency in the data-access layer. Other data-access modules only depend on createApiError which abstracts the HTTP layer.
Fix: Remove the direct HTTPException import. Refactor the check at line 712 to use a utility function (e.g., isApiError) that checks if the error is already created by createApiError.
Refs:
- agentDuplicate.ts:4 — correct pattern using
createApiErroronly
🟡 2) packages/agents-core/src/data-access/manage/agentImport.ts:148-244 Sequential dependency loading could be parallelized
Issue: loadReferencedSourceDependencies loads each dependency type sequentially using for...of with await. This results in N sequential database queries.
Why: Per AGENTS.md: "Parallelize database operations using Promise.all() instead of sequential await calls". With many dependencies, this adds significant latency.
Fix: Use Promise.all() for dependencies of the same type:
const toolResults = await Promise.all(
[...referencedDependencyIds.toolIds].map(toolId =>
getToolById(params.sourceDb)({...})
)
);Refs:
🟡 3) new-agent-item.tsx:44 Radio selection state not reset when dialog closes
Issue: The value state initialized to '' persists across dialog open/close cycles.
Why: When user opens dialog, selects 'duplicate', closes without completing, then reopens — the duplicate section may still be showing, creating inconsistent UX.
Fix: Reset state when dialog closes:
useEffect(() => {
if (!open) setValue('');
}, [open]);Inline Comments:
- 🟡 Minor:
duplicate-agent-section.tsx:44DuplicatebuildWarningSummaryfunction (see inline) - 🟡 Minor:
index.ts:7Missing agentPortability export (see inline)
💭 Consider (2) 💭
💭 1) new-agent-item.tsx:52 RadioGroup missing accessible name
Add aria-label="Agent creation method" to the RadioGroup for screen reader accessibility (WCAG 2.1 SC 1.3.1).
💭 2) duplicate-agent-section.tsx:247 + import-agent-section.tsx:326 Decorative icons should be hidden from assistive tech
Add aria-hidden="true" to AlertTriangle icons to prevent redundant screen reader announcements.
Inline Comments:
- 💭 Consider:
new-agent-item.tsx:52RadioGroup aria-label (see inline) - 💭 Consider:
duplicate-agent-section.tsx:247AlertTriangle aria-hidden (see inline)
💡 APPROVE WITH SUGGESTIONS
Summary: This PR adds well-structured agent duplication and cross-project import functionality with good transaction handling and warning systems. There are 3 Major issues worth addressing:
- API key authorization bypass — API keys skip project-level permission checks on the source project. This may be intentional (verify and document) or needs an explicit check.
- TOCTOU race condition —
duplicateFullAgentServerSidelacks transaction wrapping, inconsistent with the import flow - Test coverage gaps — Multiple error paths and conflict scenarios lack test coverage
The inline comments include specific fixes. The accessibility suggestions in "Consider" are optional but would improve the experience for assistive technology users.
Discarded (5)
| Location | Issue | Reason Discarded |
|---|---|---|
agentImport.ts:265-293 |
Credential warnings may leak credential IDs | Acceptable: caller already has view access to source project; IDs are opaque identifiers |
agent.ts:72-98 |
Test environment bypasses authorization | By design: test mode intentionally skips auth for testing; ENVIRONMENT env var separation is standard |
agentPortability.ts |
No direct unit tests | Indirect coverage via integration tests is sufficient for these pure transformation functions |
duplicate-agent-section.tsx:172 |
Dual error handling layers | Defensive programming: outer catch provides safety net for unexpected errors, not harmful |
schemas.ts:2868 |
Inline warning taxonomy definitions | New warning category; acceptable to define inline rather than reference existing taxonomy |
Reviewers (6)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-standards |
3 | 2 | 0 | 0 | 1 | 0 | 0 |
pr-review-security-iam |
5 | 1 | 0 | 0 | 0 | 0 | 4 |
pr-review-tests |
7 | 1 | 0 | 0 | 0 | 0 | 6 |
pr-review-frontend |
9 | 1 | 2 | 0 | 2 | 0 | 4 |
pr-review-consistency |
5 | 1 | 0 | 0 | 1 | 0 | 3 |
pr-review-errors |
6 | 0 | 0 | 0 | 0 | 0 | 6 |
| Total | 35 | 6 | 2 | 0 | 4 | 0 | 23 |
Note: High discard rate reflects appropriate filtering — many findings were valid observations but not actionable in PR scope or were acceptable design decisions.
|
|
||
| if (process.env.ENVIRONMENT !== 'test') { | ||
| const userId = c.get('userId'); | ||
| const tenantId = c.get('tenantId'); |
There was a problem hiding this comment.
🟠 MAJOR: Variable shadowing causes tenantId mismatch in authorization check
Issue: Line 74 declares const tenantId = c.get('tenantId') which shadows the outer tenantId from line 61's c.req.valid('param'). When canViewProject is called (line 84-88), it uses the inner tenantId from context instead of the validated route param.
Why: This could cause authorization to be checked against the wrong tenant if c.get('tenantId') differs from the route parameter. The param tenantId is validated; the context value may not be.
Fix:
| const tenantId = c.get('tenantId'); | |
| const contextTenantId = c.get('tenantId'); |
Then update line 76 to check contextTenantId for null, and line 86 to use the outer tenantId from params for canViewProject.
Refs:
- agents-api/src/domains/manage/routes/agent.ts:61 — outer tenantId from params
| onSuccess?: () => void; | ||
| } | ||
|
|
||
| function buildWarningSummary(warnings: ImportAgentWarning[]) { |
There was a problem hiding this comment.
🟡 Minor: Duplicate helper function violates DRY
Issue: buildWarningSummary is defined identically in both duplicate-agent-section.tsx (line 44) and import-agent-section.tsx (line 43).
Why: Any bug fix or improvement to warning summary logic needs to be applied in two places, risking the implementations drifting apart.
Fix: Extract to a shared utility:
// lib/utils/agent-warnings.ts
export function buildWarningSummary(warnings: ImportAgentWarning[]): string { ... }Refs:
- import-agent-section.tsx:43 — duplicate definition
|
|
||
| // Config data access (Doltgres - versioned) | ||
| export * from './manage/agentDuplicate'; | ||
| export * from './manage/agentFull'; |
There was a problem hiding this comment.
🟡 Minor: Missing export for agentPortability module
Issue: agentDuplicate.ts and agentImport.ts are exported, but agentPortability.ts (which contains shared helpers buildCopiedAgentDefinition and collectReferencedDependencyIds) is not exported from the index.
Why: Breaks the pattern where all data-access modules are accessible via @inkeep/agents-core exports. External consumers cannot import the portability helpers.
Fix:
| export * from './manage/agentFull'; | |
| export * from './manage/agentDuplicate'; | |
| export * from './manage/agentImport'; | |
| export * from './manage/agentPortability'; |
Refs:
- agentPortability.ts — the missing module
| <DialogDescription>Create a blank agent or copy an existing one.</DialogDescription> | ||
| </DialogHeader> | ||
| <div className="pt-6"> | ||
| <RadioGroup value={value} onValueChange={setValue}> |
There was a problem hiding this comment.
💭 Consider: Add accessible name to RadioGroup
Issue: The RadioGroup lacks an aria-label to describe the choice being made to screen readers.
Why: WCAG 2.1 requires form controls to have accessible names. Screen reader users navigating the form may not understand what choice the radio buttons represent.
Fix:
| <RadioGroup value={value} onValueChange={setValue}> | |
| <RadioGroup value={value} onValueChange={setValue} aria-label="Agent creation method"> |
Refs:
|
|
||
| {selectedAgent && isImportingFromAnotherProject && ( | ||
| <Alert variant="warning"> | ||
| <AlertTriangle /> |
There was a problem hiding this comment.
💭 Consider: Add aria-hidden to decorative icon
Issue: The AlertTriangle icon is decorative (the AlertTitle provides semantic meaning) but may be announced by screen readers.
Why: Screen readers may redundantly announce "AlertTriangle image" before the alert text, adding noise for assistive technology users.
Fix:
| <AlertTriangle /> | |
| <AlertTriangle aria-hidden="true" /> |
Refs:
- Same pattern needed in
import-agent-section.tsx:326
There was a problem hiding this comment.
PR Review Summary
(0) New Issues | (6) Pending from Prior Review | Risk: Medium
Delta Review: This review covers only the changes since the prior automated review (commits
7bffe558..98f297a6).
Delta Changes Analysis
The changes since the last review are minimal React Compiler cleanups:
| File | Change | Assessment |
|---|---|---|
duplicate-agent-section.tsx |
Removed 'use memo'; directive |
✅ Correct — React Compiler handles optimization |
import-agent-section.tsx |
Removed 'use memo';, useMemo import, inlined filter |
✅ Correct — aligns with React Compiler adoption |
new-agent-item.tsx |
Added className="sm:max-w-xl" to dialog |
✅ Minor styling improvement |
No new issues introduced in the delta. These changes align with the codebase-wide React Compiler migration (#1474, #2987).
🕐 Pending Recommendations (6)
The following issues from the prior review remain unaddressed:
- 🟠
agent.ts:83API key users bypass source project authorization check - 🟠
agentDuplicate.ts:44-54TOCTOU race condition — existence check and create not wrapped in transaction - 🟠
testsMissing test coverage for race conditions and dependency conflicts beyond tools - 🟡
agentImport.ts:1HTTPException import breaks data-access layer boundary - 🟡
agentImport.ts:148-244Sequential dependency loading could be parallelized - 🟡
duplicate-agent-section.tsx:44buildWarningSummaryduplicated across two components
Additionally, from the pullfrog review:
- 🟡
agent.ts:70+agentImport.ts:639DuplicatesourceProjectId === projectIdguard in both handler and DAL
💡 APPROVE WITH SUGGESTIONS
Summary: The delta changes are clean — React Compiler cleanup is appropriate and no new issues were introduced. However, 6 issues from the prior review remain unaddressed, including 3 Major findings (API key authorization bypass, TOCTOU race condition, missing test coverage). These should be addressed before or shortly after merging.
Reviewers (1)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
orchestrator (delta) |
3 | 0 | 0 | 0 | 0 | 6 | 0 |
| Total | 3 | 0 | 0 | 0 | 0 | 6 | 0 |
Note: Delta review — prior findings carried forward as Pending Recommendations.
Ito Test Report ❌19 test cases ran. 1 failed, 18 passed. Across 19 copy/import test cases, 18 passed and 1 failed, indicating the duplicate/import UI and API flows are broadly reliable with expected redirects and 201 responses, strong validation and authorization/anti-enumeration behavior, safe handling of malicious input, preserved conversation-history settings, trigger exclusion on copies, credential-disconnect warnings, dialog/mobile usability, and clean stale-selection/interruption handling. The primary confirmed defect is a high-severity concurrency race in rapid double-submit scenarios where two near-simultaneous requests for the same newAgentId can surface an intermittent 500 instead of a deterministic conflict response, even though normal existing-ID conflict handling otherwise worked and did not mutate existing agents. ❌ Failed (1)
|
| Category | Summary | Screenshot |
|---|---|---|
| Adversarial | At 390x844 viewport, copy dialog controls remained visible/tappable and same-project duplicate completed successfully to dup-mobile-01 without layout-caused misselection. |  |
| Adversarial | Authorization boundary is correctly enforced: inaccessible source-project import returns anti-enumeration not-found behavior. |  |
| Adversarial | Invalid format ID was rejected, and malicious newAgentName was rendered as escaped literal text without script execution. |  |
| Adversarial | Refresh/back-forward interruption recovered cleanly with no false-success state or persisted artifacts. |  |
| Edge | Submitting copy with newAgentId equal to source-agent-main was rejected with a bad-request-style validation toast and no creation/redirect occurred. |  |
| Edge | UI displayed invalid-ID validation and prevented copy creation with malformed ID; malicious newAgentName payload remained inert. |  |
| Edge | Verified import endpoint rejects same-project tampered payload with HTTP 400 bad_request and explicit guidance to use duplicate flow; no import created. |  |
| Edge | Import submission after deleting the selected source agent returns not_found and creates no target agent. |  |
| Edge | Duplicate/import to an existing target ID returned conflict without mutating the existing agent. |  |
| Edge | Original BLOCKED result was a harness/tooling artifact (watchdog kill after silent hung command), not an application defect. Re-investigation plus source validation confirms import is unavailable without an alternate project because import path only executes when sourceProjectId differs from current project and alternate projects exist. |  |
| Edge | After setting non-default source project/agent and custom name/id, closing without submit and reopening reset the dialog to defaults with cleared fields. |  |
| Logic | Source, duplicate, and imported agents preserved identical sub-agent conversation history configuration values. |  |
| Logic | Source retained webhook and scheduled triggers while copied/imported agents in target project had no triggers. |  |
| Logic | Cross-project copy displayed credential disconnect warning and completed import to the target agent details page. |  |
| Happy-path | Copied source-agent-main in same project with new ID dup-ui-route1-02; copy succeeded and redirected to new agent details. |  |
| Happy-path | Cross-project import via UI copy flow succeeded and opened imported agent details in the target project. |  |
| Happy-path | Authenticated duplicate endpoint returned 201 with copied payload and expected id dup-api-route3-02; follow-up GET succeeded. |  |
| Happy-path | Import API returned 201 with copied agent data and warnings array; imported agent was retrievable via GET /full. |  |
Commit: 98f297a
Tell us how we did: Give Ito Feedback
1 participant
Agent Duplicate & Cross-Project Import
Adds two new agent copy flows — same-project duplication and cross-project import — across the API, data access layer, and dashboard UI.
New API Endpoints
POST/agents/{agentId}/duplicatePOST/agents/importCore Logic (
agents-core)agentDuplicate.ts—duplicateFullAgentServerSide: clones a full agent definition (viabuildCopiedAgentDefinition) within the same project. Validates the new ID differs from the source and that no agent with that ID already exists.agentImport.ts—importFullAgentServerSide: copies an agent from a source project into a target project inside a transaction. Loads all referenced project-scoped dependencies (tools, external agents, data components, artifact components, functions, skills) from the source, then ensures each exists in the target — creating them if missing, erroring on conflicts with differing configuration. Normalizes resource shapes for stable equality comparison. Producescredential_missingwarnings when imported tools or external agents reference credentials absent in the target project.agentPortability.ts— Shared helpers:buildCopiedAgentDefinitionstrips non-portable root keys (tools,externalAgents,teamAgents,functions,triggers), normalizes transfer/delegate targets, and re-validates throughvalidateAndTypeAgentData.collectReferencedDependencyIdswalks sub-agents to collect all dependency IDs needed for cross-project import.Validation Schemas
New Zod schemas and inferred types in
agents-core:DuplicateAgentRequestSchema—{ newAgentId, newAgentName? }ImportAgentRequestSchema—{ sourceProjectId, sourceAgentId, newAgentId, newAgentName? }ImportAgentWarningSchema—{ code: 'credential_missing', resourceType, resourceId, credentialReferenceId }ImportAgentResponseSchema—{ data, warnings }Authorization
editpermission on the target project.viewaccess on the source project (viacanViewProject).withRef/getProjectMainResolvedRef.Dashboard UI (
agents-manage-ui)new-agent-item.tsx— "New Agent" dialog now presents a radio group: Create blank agent or Copy existing agent.duplicate-agent-section.tsx— Renders the copy form. When the selected source project matches the current project, calls the duplicate endpoint. When a different project is selected, calls the import endpoint. Displays post-importcredential_missingwarnings.import-agent-section.tsx— Standalone import form component for the cross-project flow with project/agent combobox selectors and auto-prefilled ID/name.duplicateAgentAction,importAgentAction) and API client functions added for both flows.FieldUI components (Field,FieldContent,FieldTitle,FieldDescription,FieldLabel) used in the radio group layout.Documentation
agents-docs/content/visual-builder/sub-agents.mdx— New "Copying an Agent" section documenting both same-project and cross-project copy flows, including behavior around triggers, credentials, and team-agent limitations.Tests
agentDuplicate.test.ts— Unit tests for same-project duplication (success, ID collision, same-ID rejection).agentImport.test.ts— Unit tests for cross-project import covering dependency recreation, conflict detection, credential warnings, team-agent rejection, and same-project guard.agent.test.ts— Route-level CRUD tests for both/duplicateand/importendpoints.Claude Opus｜ 𝕏