Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 84 additions & 1 deletion CubeOps/internal/service/agenthub.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,19 @@ type CubeMasterClient interface {
// SDKInstanceType is the CubeMaster instance_type used for AgentHub sandboxes.
const SDKInstanceType = "cubebox"

// defaultAgentTemplateID is the last-resort template identifier CreateInstance
// uses when the caller supplies neither a snapshot nor a templateId and no
// AgentHub template is registered to default to.
//
// Nothing in this repository provisions a template under this identifier: it is
// not a tpl-/snap- id, so CubeMaster resolves it through GetTemplateByAlias,
// and it only resolves on installs where an operator happened to claim it as a
// template alias. Reaching it therefore means "no template is registered",
// which CreateInstance reports as such instead of surfacing CubeMaster's
// not-found. Kept as a fallback rather than dropped so that installs which do
// carry the alias keep working.
const defaultAgentTemplateID = "wecom-ds-openclaw"

// ── Error ───────────────────────────────────────────────────────────────────

// Error is the service-layer error type. It carries an HTTP status code so
Expand Down Expand Up @@ -105,6 +118,13 @@ func wrapCMError(err error) *Error {
}
}

// isCMNotFound reports whether err is a CubeMaster not-found (130404). Callers
// that need the full status mapping should use wrapCMError instead.
func isCMNotFound(err error) bool {
Comment thread
ENCHIGO marked this conversation as resolved.
var cmErr *cubemaster.CMError
return errors.As(err, &cmErr) && cmErr.IsNotFound()
}

// ── AgentStore interface ────────────────────────────────────────────────────

// AgentStore is the subset of *store.Store that AgentHubService depends on.
Expand All @@ -123,6 +143,7 @@ type AgentStore interface {
GetAgentSnapshot(ctx context.Context, agentID, snapshotID string) (*store.AgentSnapshot, error)
DeleteAgentSnapshot(ctx context.Context, agentID, snapshotID string) error
GetAgentTemplate(ctx context.Context, templateID string) (*store.AgentTemplate, error)
ListAgentTemplates(ctx context.Context, limit, offset int) ([]store.AgentTemplate, error)
RecordOperation(ctx context.Context, agentID, sandboxID, operationType, status, errMsg string) error
LatestHealthySnapshot(ctx context.Context, agentID string) (string, error)
SetBaseSnapshotID(ctx context.Context, agentID, snapshotID string) error
Expand Down Expand Up @@ -391,6 +412,55 @@ type CreateInstanceResult struct {
Instance *store.AgentInstance
}

// defaultTemplateID picks the template CreateInstance uses when the caller
// omits templateId: the recommended registered template if there is one,
// otherwise the most recently registered one (ListAgentTemplates orders by
// created_at DESC). The recommended flag is what the Dashboard sets on every
Comment thread
ENCHIGO marked this conversation as resolved.
Outdated
// template it registers from the market.
//
// The preference is exact rather than window-limited: pages are walked until
// one comes back short, so a recommended template still wins when newer
// non-recommended ones were registered after it. Any realistic registry fits
// in the first page, so this is one query in practice.
//
// none is true only when the registry was read successfully and holds nothing.
// A failed read returns none=false — the caller must not report "nothing is
// registered" on the strength of a listing that never happened. Either way the
// returned id is then defaultAgentTemplateID, which the caller still passes to
// CubeMaster so installs carrying that alias keep working.
func (s *AgentHubService) defaultTemplateID(ctx context.Context) (id string, none bool) {
mostRecent := ""
for offset := 0; ; offset += store.MaxListLimit {
Comment thread
ENCHIGO marked this conversation as resolved.
Outdated
page, err := s.Store.ListAgentTemplates(ctx, store.MaxListLimit, offset)
if err != nil {
logging.G(ctx).Warnf("failed to list agent templates while choosing a default: %v", err)
break
}
for _, tmpl := range page {
if tmpl.Recommended {
return tmpl.TemplateID, false
}
}
if mostRecent == "" && len(page) > 0 {
mostRecent = page[0].TemplateID
}
if len(page) < store.MaxListLimit {
// Short page: the registry is exhausted and held no recommended
// template, so mostRecent (if any) is the answer.
if mostRecent != "" {
return mostRecent, false
}
return defaultAgentTemplateID, true
}
}
// Listing failed part-way. Use anything already seen, and do not claim the
// registry is empty.
if mostRecent != "" {
return mostRecent, false
}
return defaultAgentTemplateID, false
}

// CreateInstance orchestrates the full agent creation flow:
//
// 1. Resolve LLM config + domain + egress network config from settings.
Expand Down Expand Up @@ -431,14 +501,19 @@ func (s *AgentHubService) CreateInstance(ctx context.Context, req CreateInstance
snapshotID := strings.TrimSpace(req.SnapshotID)
rootfsSourceType := "template"
rootfsSourceID := ""
// Set when the request named no template and the registry was read and
// found empty, so the unprovisioned defaultAgentTemplateID was used; lets
// the CreateSandbox error below name the real problem. Deliberately not
// set when the registry could not be read — see defaultTemplateID.
noTemplateRegistered := false
if snapshotID != "" {
rootfsSourceType = "snapshot"
rootfsSourceID = snapshotID
} else {
if req.TemplateID != "" {
rootfsSourceID = req.TemplateID
} else {
rootfsSourceID = "wecom-ds-openclaw"
rootfsSourceID, noTemplateRegistered = s.defaultTemplateID(ctx)
}
}
templateID := rootfsSourceID
Expand Down Expand Up @@ -535,6 +610,14 @@ func (s *AgentHubService) CreateInstance(ctx context.Context, req CreateInstance
// --- Create sandbox ---
sandboxResp, err := s.CM.CreateSandbox(ctx, cmReq)
if err != nil {
// The caller named no template, none is registered, and CubeMaster
// could not resolve the fallback either. Report the missing
// registration rather than a not-found for an identifier the caller
// never supplied.
if noTemplateRegistered && isCMNotFound(err) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The rewrite fires on any CubeMaster not-found once the registry was read empty — not specifically a template-resolution failure. In the empty-registry request the only template-ish input is the fallback id, but CreateSandbox also carries instance_type, network_type: "tap", and distribution_scope; a 130404 raised for one of those (e.g. a misconfigured network) would be reported as "no agent template is registered", masking the real cause. Since the error message in the repro already names the fallback identifier, a tighter predicate such as strings.Contains(err.Error(), defaultAgentTemplateID) would scope the rewrite to the case this PR is about while still being strictly better than the old 502. Not blocking — the current broad check is a reasonable default if you'd rather always err toward the actionable hint.

return nil, NewBadRequest("no agent template is registered: register one from the template market " +
"(POST /agenthub/templates/market), or pass templateId explicitly")
}
return nil, NewBadGateway("failed to create sandbox: " + err.Error())
}
var sbResult struct {
Expand Down
234 changes: 234 additions & 0 deletions CubeOps/internal/service/agenthub_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type fakeAgentStore struct {
getAgentSnapshot func(ctx context.Context, agentID, snapshotID string) (*store.AgentSnapshot, error)
deleteAgentSnapshot func(ctx context.Context, agentID, snapshotID string) error
getAgentTemplate func(ctx context.Context, templateID string) (*store.AgentTemplate, error)
listAgentTemplates func(ctx context.Context, limit, offset int) ([]store.AgentTemplate, error)
recordOperation func(ctx context.Context, agentID, sandboxID, operationType, status, errMsg string) error
latestHealthySnapshot func(ctx context.Context, agentID string) (string, error)
setBaseSnapshotID func(ctx context.Context, agentID, snapshotID string) error
Expand Down Expand Up @@ -109,6 +110,12 @@ func (f *fakeAgentStore) GetAgentTemplate(ctx context.Context, templateID string
}
return f.getAgentTemplate(ctx, templateID)
}
func (f *fakeAgentStore) ListAgentTemplates(ctx context.Context, limit, offset int) ([]store.AgentTemplate, error) {
if f.listAgentTemplates == nil {
return nil, nil // default: no template registered
}
return f.listAgentTemplates(ctx, limit, offset)
}
func (f *fakeAgentStore) RecordOperation(ctx context.Context, agentID, sandboxID, operationType, status, errMsg string) error {
if f.recordOperation == nil {
return nil
Expand Down Expand Up @@ -346,6 +353,233 @@ func TestCreateInstance_ValidationErrors(t *testing.T) {
}
}

// llmKeyStore returns a fakeAgentStore whose only wired method resolves the
// LLM API key, which CreateInstance requires before it reaches CubeMaster.
func llmKeyStore() *fakeAgentStore {
return &fakeAgentStore{
getSetting: func(_ context.Context, key string) (string, error) {
if key == "llm_api_key" {
return "test-key", nil
}
return "", nil
},
}
}

// rootfsSourceID returns the resolved rootfs source id CreateInstance sent to
// CubeMaster, which BuildCreateSandboxRequest carries as a label.
func rootfsSourceID(t *testing.T, cm *fakeServiceCM) string {
t.Helper()
if cm.createSandboxBody == nil {
t.Fatal("CreateSandbox was not called")
}
labels, _ := cm.createSandboxBody["labels"].(map[string]interface{})
id, _ := labels["agenthub.rootfs_source_id"].(string)
return id
}

// TestCreateInstance_DefaultTemplateSelection verifies which template
// CreateInstance uses when the request names neither a snapshot nor a
// templateId: the recommended registered template if there is one, else the
// most recently registered one (ListAgentTemplates orders created_at DESC).
// Before this, the request always went out with the hardcoded
// defaultAgentTemplateID, which nothing provisions.
func TestCreateInstance_DefaultTemplateSelection(t *testing.T) {
tests := []struct {
name string
templates []store.AgentTemplate
want string
}{
{
name: "prefers the recommended template over a newer one",
templates: []store.AgentTemplate{
{TemplateID: "tpl-newest", Recommended: false},
{TemplateID: "tpl-recommended", Recommended: true},
},
want: "tpl-recommended",
},
{
name: "falls back to the most recent when none is recommended",
templates: []store.AgentTemplate{
{TemplateID: "tpl-newest", Recommended: false},
{TemplateID: "tpl-older", Recommended: false},
},
want: "tpl-newest",
},
{
name: "uses the built-in identifier when nothing is registered",
templates: nil,
want: defaultAgentTemplateID,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cm := &fakeServiceCM{}
st := llmKeyStore()
st.listAgentTemplates = func(_ context.Context, _, _ int) ([]store.AgentTemplate, error) {
return tt.templates, nil
}
svc := newTestService(st, cm)

if _, err := svc.CreateInstance(context.Background(), CreateInstanceRequest{
Name: "my-agent",
Engine: "openclaw",
}); err != nil {
t.Fatalf("CreateInstance returned error: %v", err)
}
if got := rootfsSourceID(t, cm); got != tt.want {
t.Errorf("rootfs_source_id = %q, want %q", got, tt.want)
}
})
}
}

// TestCreateInstance_RecommendedBeyondFirstPage verifies that the recommended
// preference is exact rather than window-limited: a recommended template that
// only appears on a later page still wins over the newest one.
func TestCreateInstance_RecommendedBeyondFirstPage(t *testing.T) {
full := make([]store.AgentTemplate, store.MaxListLimit)
for i := range full {
full[i] = store.AgentTemplate{TemplateID: "tpl-filler", Recommended: false}
}
full[0].TemplateID = "tpl-newest"

cm := &fakeServiceCM{}
st := llmKeyStore()
st.listAgentTemplates = func(_ context.Context, limit, offset int) ([]store.AgentTemplate, error) {
if offset == 0 {
return full, nil // exactly one full page, so a second is fetched
}
return []store.AgentTemplate{{TemplateID: "tpl-recommended", Recommended: true}}, nil
}
svc := newTestService(st, cm)

if _, err := svc.CreateInstance(context.Background(), CreateInstanceRequest{
Name: "my-agent",
Engine: "openclaw",
}); err != nil {
t.Fatalf("CreateInstance returned error: %v", err)
}
if got := rootfsSourceID(t, cm); got != "tpl-recommended" {
t.Errorf("rootfs_source_id = %q, want tpl-recommended", got)
}
}

// TestCreateInstance_TemplateListingFailureIsNotReportedAsUnregistered
// verifies that a failed registry read is not turned into "no agent template
// is registered": the listing never happened, so CubeMaster's own error is
// what the caller gets.
func TestCreateInstance_TemplateListingFailureIsNotReportedAsUnregistered(t *testing.T) {
cm := &fakeServiceCM{
createSandboxErr: &cubemaster.CMError{RetCode: 130404, RetMsg: "template not found"},
}
st := llmKeyStore()
st.listAgentTemplates = func(_ context.Context, _, _ int) ([]store.AgentTemplate, error) {
return nil, errors.New("dial tcp: connection refused")
}
svc := newTestService(st, cm)

_, err := svc.CreateInstance(context.Background(), CreateInstanceRequest{
Name: "my-agent",
Engine: "openclaw",
})
var svcErr *Error
if !errors.As(err, &svcErr) {
t.Fatalf("error is not *service.Error: %v", err)
}
if svcErr.Status != 502 {
t.Errorf("status = %d, want 502", svcErr.Status)
}
if strings.Contains(svcErr.Message, "no agent template is registered") {
t.Errorf("message = %q, should not claim an empty registry when the listing failed", svcErr.Message)
}
}

// TestCreateInstance_ExplicitTemplateIDWins verifies that an explicit
// templateId is used as-is and the registered-template lookup is skipped.
func TestCreateInstance_ExplicitTemplateIDWins(t *testing.T) {
cm := &fakeServiceCM{}
st := llmKeyStore()
listed := false
st.listAgentTemplates = func(_ context.Context, _, _ int) ([]store.AgentTemplate, error) {
listed = true
return []store.AgentTemplate{{TemplateID: "tpl-recommended", Recommended: true}}, nil
}
svc := newTestService(st, cm)

if _, err := svc.CreateInstance(context.Background(), CreateInstanceRequest{
Name: "my-agent",
Engine: "openclaw",
TemplateID: "tpl-explicit",
}); err != nil {
t.Fatalf("CreateInstance returned error: %v", err)
}
if got := rootfsSourceID(t, cm); got != "tpl-explicit" {
t.Errorf("rootfs_source_id = %q, want tpl-explicit", got)
}
if listed {
t.Error("ListAgentTemplates should not be consulted when templateId is explicit")
}
}

// TestCreateInstance_NoTemplateRegisteredReportsMissingRegistration verifies
// that when no template is registered and CubeMaster cannot resolve the
// built-in identifier either, the caller gets an actionable 400 instead of a
// 502 naming an identifier they never supplied.
func TestCreateInstance_NoTemplateRegisteredReportsMissingRegistration(t *testing.T) {
cm := &fakeServiceCM{
createSandboxErr: &cubemaster.CMError{
RetCode: 130404,
RetMsg: `failed to resolve template identifier "` + defaultAgentTemplateID + `": template not found`,
},
}
svc := newTestService(llmKeyStore(), cm)

_, err := svc.CreateInstance(context.Background(), CreateInstanceRequest{
Name: "my-agent",
Engine: "openclaw",
})
var svcErr *Error
if !errors.As(err, &svcErr) {
t.Fatalf("error is not *service.Error: %v", err)
}
if svcErr.Status != 400 {
t.Errorf("status = %d, want 400", svcErr.Status)
}
if !strings.Contains(svcErr.Message, "no agent template is registered") {
t.Errorf("message = %q, want it to name the missing registration", svcErr.Message)
}
if strings.Contains(svcErr.Message, defaultAgentTemplateID) {
t.Errorf("message = %q, should not surface the built-in identifier to the caller", svcErr.Message)
}
}

// TestCreateInstance_ExplicitTemplateNotFoundStaysBadGateway guards the
// narrowness of the case above: a not-found for a template the caller did name
// is still reported as-is, not rewritten into the registration hint.
func TestCreateInstance_ExplicitTemplateNotFoundStaysBadGateway(t *testing.T) {
cm := &fakeServiceCM{
createSandboxErr: &cubemaster.CMError{RetCode: 130404, RetMsg: "template not found"},
}
svc := newTestService(llmKeyStore(), cm)

_, err := svc.CreateInstance(context.Background(), CreateInstanceRequest{
Name: "my-agent",
Engine: "openclaw",
TemplateID: "tpl-typo",
})
var svcErr *Error
if !errors.As(err, &svcErr) {
t.Fatalf("error is not *service.Error: %v", err)
}
if svcErr.Status != 502 {
t.Errorf("status = %d, want 502", svcErr.Status)
}
if strings.Contains(svcErr.Message, "no agent template is registered") {
t.Errorf("message = %q, should not claim a missing registration", svcErr.Message)
}
}

// TestCreateInstance_ApplyFailureCompensates verifies that when the
// OpenClaw apply step fails after the sandbox was already created,
// CompensateDeleteSandbox is invoked to clean up the orphan sandbox.
Expand Down
Loading