Skip to content

fix(server): ACE-39 reject publishing an Agent Template with no components - #2249

Open
Gandy2025 wants to merge 1 commit into
mainfrom
fix/agent-template-publish-requires-components
Open

fix(server): ACE-39 reject publishing an Agent Template with no components#2249
Gandy2025 wants to merge 1 commit into
mainfrom
fix/agent-template-publish-requires-components

Conversation

@Gandy2025

Copy link
Copy Markdown
Contributor

Summary

  • publishAgentTemplate re-parsed the stored payload and verified every Skill bundle projection, but never checked the component count. agentTemplatePayloadSchema intentionally allows an empty components array (drafts start empty), so a component-less draft published cleanly and showed up in the public catalog like any other Template.
  • The only check lived downstream in lockAndVerifyTemplate (agent-template-adoption.ts), which throws has no components and cannot be adopted. That pushed the error to the worst possible place: the publisher saw a successful publish, the catalog looked fine, and the failure only surfaced once a real user clicked Use.
  • Publish now rejects an empty payload inside the transaction with a ConflictError (409) worded to match the adoption-side check, so the publisher gets the feedback at publish time and the Template stays draft. Non-empty publishes are unchanged.

Fixes ACE-39.

Validation

  • pnpm check
  • pnpm typecheck
  • pnpm test

packages/server: 3125 tests pass, including the new case. Full-suite runs on this machine also surfaced timeout-only failures in @first-tree/client, @first-tree/web, and @first-tree/skill-evals; each passes in isolation. The one exception is client/src/__tests__/pi-handler.test.ts > exhausts active version-gate timeout..., which fails identically on the unmodified base commit (54bcb109c) because it needs a working pi binary on the host — pre-existing, unrelated to this change.

Change Surface

  • apps/cli public CLI or help output
  • tree onboarding / binding / inspection behavior
  • shipped or planned skill topology
  • docs or contributor-facing repository metadata
  • CI / packaging / release plumbing

Notes

  • package or install behavior changes: none.
  • docs or tests updated to match: added rejects publishing a Template with no components and leaves it draft to packages/server/src/__tests__/agent-templates.test.ts — asserts the 409, that the row stays draft with an unchanged updatedAt token, and that the same draft publishes fine once it carries a component. No docs reference publish preconditions.
  • follow-up work: the shared schema still allows components: [] by design so drafts can be created empty; publish is the right gate. If empty drafts turn out to have no use, .min(1) on the write-source schema would be a separate, larger contract change.

publishAgentTemplate re-parsed the stored payload and verified every Skill
bundle projection, but never checked the component count. Since
agentTemplatePayloadSchema allows an empty components array, a
component-less draft published cleanly and appeared in the public catalog;
the failure only surfaced downstream in lockAndVerifyTemplate, once a real
user clicked Use.

Publish now rejects an empty payload with a 409 whose wording matches the
adoption-side check, so the publisher gets the feedback at publish time and
the template stays draft.

Co-authored-by: multica-agent <github@multica.ai>

@yuezengwu yuezengwu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

实现目标:把“模板至少包含一个组件”的校验从采用阶段前移到发布阶段,避免空模板进入公开目录后才在用户点击 Use 时失败。

核心改动:publishAgentTemplate 在重新解析存储 payload 后、锁定 Skill bundle 前检查 components.length;为空时抛出 ConflictError,因此事务保持 draft 状态且不推进 updatedAt。新增回归测试覆盖 409、状态与并发 token 不变,以及补齐组件后可正常发布。

Review:检查了状态转换、事务边界、公开目录与 adoption 侧的同类防线;实现范围正确,未发现 blocker。此 PR 不涉及数据库 schema、migration 或核心数据结构变更。按 review 约定未额外运行测试。

@baixiaohang baixiaohang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommendation: approve

  • Rationale: The publish boundary now rejects component-less definitions atomically while preserving legal empty drafts and the adoption-side defense.

Risk level: B-high

  • Path baseline: packages/server/src/services/agent-templates.ts changes the core Agent Template service.
  • Semantic lift: none beyond the B-high path baseline.

PR summary

  • Author / repo: Gandy2025 / agent-team-foundation/first-tree
  • Problem: A publisher could make an empty Agent Template public, leaving the first real user to discover that it could not be used.
  • Approach: Publishing now returns a 409 before any state mutation when the stored payload has no components; the Template remains a draft and can be published after a component is added. The shared payload schema still permits empty drafts, and adoption retains its defensive check.
  • Impacted modules: Agent Template catalog service and server regression tests.

Review findings
✅ 1. The guard runs under the locked-row transaction before status or timestamp mutation, preserving atomic failure semantics. [R4 / packages/server/src/services/agent-templates.ts:409]
✅ 2. The lifecycle-specific publish guard avoids incorrectly tightening the draft payload schema, while the adoption guard remains as defense in depth. [R3 / packages/server/src/services/agent-templates.ts:409]
✅ 3. The regression covers the 409, unchanged draft state and concurrency token, and successful publication after adding a component. [packages/server/src/__tests__/agent-templates.test.ts:344]
⚠️ 4. This prevents new invalid publications but does not remediate any component-less active rows that may predate the fix; those would need a separate data audit or cleanup if they exist. [R5]

Action taken

  • Approved at head 1e1512b8a7ac5271b1144d527613eaf13459ba51.

Per review policy, I did not run tests locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants