fix(cubeops): default AgentHub creates to a registered template

[ENCHIGO]

Summary

AgentHubService.CreateInstance fell back to the hardcoded template identifier "wecom-ds-openclaw" whenever the request carried neither a snapshot nor a templateId. Nothing in this repository provisions a template under that identifier — it is not a tpl-/snap- id, so CubeMaster resolves it through GetTemplateByAlias, and it only resolves on installs where an operator claimed that alias by hand. On a self-hosted install every such create fails with

failed to create sandbox: cubemaster error 130404: failed to resolve template identifier "wecom-ds-openclaw": template not found

naming an identifier the caller never supplied. The Dashboard sends exactly this request whenever its template selector has nothing to select.

Fixes #1327 — full analysis, repro and environment there.

What changed

• CreateInstance resolves its default from the registered AgentHub templates: the one an operator marked recommended if there is one, otherwise the most recently registered. Both are single bounded queries — store.GetRecommendedAgentTemplate (new) and ListAgentTemplates(1, 0).
• The literal moves into a documented defaultAgentTemplateID constant and stays as a last resort, so installs that do carry the alias are unaffected.
• When that last resort is the identifier CubeMaster failed to resolve, the caller gets a 400 naming the missing registration instead of the 502 not-found. The rewrite is scoped to that one case: a not-found for a template the caller did name still surfaces as before, and so does one after a registry read that failed rather than came back empty.
• GetRecommendedAgentTemplate / ListAgentTemplates added to the AgentStore interface — *store.Store already implemented the latter.

Behaviour

request	before	after
templateId given	used as-is	unchanged
snapshotId given	used as-is	unchanged
neither, template(s) registered	wecom-ds-openclaw → 130404	recommended if one is marked, else most recently registered
neither, none registered, alias exists on the install	resolves	unchanged — still resolves
neither, none registered, no alias	502 cubemaster error 130404: … "wecom-ds-openclaw" …	400 no agent template is registered: register one from the template market (POST /agenthub/templates/market), or pass templateId explicitly
neither, registry read failed	502 from CubeMaster	unchanged — 502, never the 400

Where recommended comes from

Correction to an earlier revision of this description, which claimed the Dashboard marks every market-registered template recommended. It does send recommended: true, but the server does not persist it: RegisterMarketTemplate (internal/handler/agenthub.go:522-560) parses the field and its INSERT omits the column, and UpsertTemplateSQL (internal/store/dialect.go:139-157) hardcodes it false on the publish path — so registration always lands recommended = 0.

The flag is set only by PATCH /agenthub/templates/{templateID}, which the Dashboard's per-template toggle calls (web/src/pages/AgentHub.tsx:482). That makes the preference an explicit operator choice rather than a side effect of registration, and it is why this PR resolves it with its own query: a marked template can sit behind any number of newer registrations.

Whether registration should persist the flag the Dashboard sends looks like a separate question — persisting it as-is would mark every market template recommended — so this PR leaves that path alone. Happy to file it separately if you would like it changed.

Testing

go test ./... passes in CubeOps/, including the dockertest store suite against MySQL 8.0.

• TestStore_GetRecommendedAgentTemplate (real MySQL) — nothing recommended after registration; a marked older row wins over a newer unmarked one; a soft-deleted recommendation does not come back
• TestCreateInstance_DefaultTemplateSelection — recommended wins; most-recent when none is marked; the built-in identifier when nothing is registered
• TestCreateInstance_RecommendedIsNotWindowLimited — the registry is not listed at all once a recommended template is found
• TestCreateInstance_ExplicitTemplateIDWins — an explicit id is used as-is and the registry is never consulted
• TestCreateInstance_NoTemplateRegisteredReportsMissingRegistration — the actionable 400, and that the built-in identifier is not leaked to the caller
• TestCreateInstance_TemplateReadFailureIsNotReportedAsUnregistered — either read failing keeps the 502, so a transient DB failure is never reported as an empty registry
• TestCreateInstance_ExplicitTemplateNotFoundStaysBadGateway — guards the narrowness of the rewrite

The 130404 in the issue was hit on a v0.6.0 single-node install; this branch is covered by the tests above and has not been deployed to that install.

Left alone deliberately

docs/guide/digital-assistant.md:17 (and the zh mirror) and CubeAPI/scripts/test-cube-api.sh:26 also reference wecom-ds-openclaw as though it exists. Those look like the same leak but they are documentation/tooling wording rather than behaviour, so they are not touched here — happy to follow up in a separate PR if you want them reworded.

Assisted-by: Claude Code:claude-opus-5

[cubesandboxbot]

Review: fix(cubeops): default AgentHub creates to a registered template (PR #1334)

AI-generated review — this is an automated assessment, not a human approval.

Verdict

The fix is sound and well-targeted. The root-cause analysis is accurate (verified against the base tree: RegisterMarketTemplate at internal/handler/agenthub.go:608 omits the recommended column, UpsertTemplateSQL at internal/store/dialect.go:149-157 hardcodes 0, and only PATCH /agenthub/templates/{templateID} sets the flag), the default-template resolution is correct and bounded, the 400 rewrite gives callers an actionable error without leaking the internal wecom-ds-openclaw identifier, and the test suite is unusually thorough — including explicit guards for the narrowness of the rewrite. I found no correctness bugs that would block merging. The points below are minor robustness/design notes, plus a couple of nits.

Verified strengths

• Default resolution is correct. GetRecommendedAgentTemplate (recommended flag, newest first) then ListAgentTemplates(1, 0) (most recent) then the documented fallback constant — both reads bounded to one row. Matches the PR's behaviour table.
• Narrow 400 rewrite. The noTemplateRegistered flag is only set when the registry was read successfully and came back empty; a failed read keeps the 502, and an explicit templateId/snapshotId never triggers the rewrite. All three guard cases are tested (TemplateReadFailureIsNotReportedAsUnregistered, ExplicitTemplateIDWins, ExplicitTemplateNotFoundStaysBadGateway).
• HTTPError change is additive and safe. The Error() string is byte-identical to the previous fmt.Errorf, so operator greps/log assertions are unaffected, and existing tests exercise only the 200-body ret_code path (unchanged).
• Interface update is complete. AgentStore gained both methods; *store.Store already had ListAgentTemplates; the service fake implements both with defaults that preserve the old behaviour for existing tests.
• PR description accuracy. The claim that registration never persists recommended is confirmed by reading the handler and dialect code.

Notes (low severity)

1. isCMNotFound HTTP branch keys on status only, not content (see inline comment at agenthub.go:138). A not-found that arrives as a non-404 HTTP status carrying ret_code: 130404 would evade the rewrite. Both shapes in the [Bug Report] AgentHub CreateInstance falls back to a hardcoded template identifier "wecom-ds-openclaw" that nothing provisions #1327 repro are covered today, so this is defensive only.

2. The 400 rewrite is scoped to "any CubeMaster not-found" rather than "template-resolution failure" (see inline comment at agenthub.go:617). When the registry is empty, the request still carries instance_type/network_type/distribution_scope, so an unrelated 130404 could be misreported as "no agent template is registered". A predicate on strings.Contains(err.Error(), defaultAgentTemplateID) would tighten it; not blocking.

3. Multiple templates can be marked recommended simultaneously. PATCH /agenthub/templates/{id} lets any template be flagged, and the Dashboard toggle (web/src/pages/AgentHub.tsx:482) doesn't unmark siblings — so an operator can end up with several recommended, and GetRecommendedAgentTemplate silently picks the newest. Deterministic, but the silent tie-break may surprise; consider unmarking others in the PATCH handler, or documenting the rule.

Nits

• defaultTemplateID runs before LLM-config resolution, so a request that is doomed to fail on a missing llm_api_key still pays two (cheap, single-row) DB queries. Could be reordered after ResolveLLMConfig for free.
• The new HTTPError type has no direct unit test in the cubemaster package for the readResponse ≥400 branch (it's covered indirectly via TestIsCMNotFoundCoversBothShapes constructing the type by hand). A round-trip test against an httptest.Server returning a non-2xx status would pin the typed-error contract.
• TestStore_GetRecommendedAgentTemplate is dockertest-gated and will skip where Docker is unavailable — fine, but worth knowing that the store-level coverage only runs in CI with Docker.

Not addressed (acknowledged in the PR description)

The wecom-ds-openclaw references in docs/guide/digital-assistant.md and CubeAPI/scripts/test-cube-api.sh remain; the PR correctly scopes itself to runtime behaviour. A follow-up to reword those would close the loop.

[ENCHIGO]

Pushed 9b83692 addressing both inline findings — replies in the threads, summary here:

1. Listing failure vs. empty registry — real defect, fixed. The none flag now means "the registry was read and held nothing"; a failed read no longer lets the code assert an empty registry, and CubeMaster's own error surfaces instead. Deliberately not mapped to a 500, so a transient listing failure cannot newly fail a create that would have worked through the alias fallback.

2. Recommended preference was window-limited — fixed exactly rather than by widening the window: pages are walked until one comes back short, in pages of MaxListLimit. Still one query for any realistic registry.

On the third item (a registered template that CubeMaster cannot resolve — e.g. its backing snapshot was deleted — still surfacing as a 502 naming a tpl-… the caller never supplied): left as-is on purpose. The rewrite in this PR is sound because the service knows exactly why that one case fails — it supplied an identifier that nothing provisions. For a registered-but-unresolvable template the service does not know the cause, and the actionable advice is different (the registration is stale, not missing), so any message it invented there would be a guess dressed as a diagnosis. If you want that case covered too, the honest shape is probably to detect it and prune or flag the stale registration rather than to reword the error — happy to do that in a follow-up if a maintainer thinks it is worth it.

Two tests added for the fixes: TestCreateInstance_RecommendedBeyondFirstPage and TestCreateInstance_TemplateListingFailureIsNotReportedAsUnregistered. go test ./... is green in CubeOps/.

[ENCHIGO]

Pushed 7c40da0. Both new findings addressed, and one of them was a false premise of mine, so flagging it at top level rather than only in the thread:

Correction. I justified the recommended-first preference by saying the Dashboard marks every market-registered template recommended. That is wrong. It sends recommended: true, but RegisterMarketTemplate (handler/agenthub.go:522-560) omits the column from its INSERT and UpsertTemplateSQL (store/dialect.go:139-157) hardcodes it false, so registration always lands recommended = 0. The PR description now says where the flag actually comes from: PATCH /agenthub/templates/{templateID}, which the Dashboard's per-template toggle calls (web/src/pages/AgentHub.tsx:482).

That makes the preference an explicit operator choice rather than a side effect of registration — real, but set out of band, so a marked template can sit behind any number of newer ones. Both findings therefore collapse into the same fix: resolve it in the database. store.GetRecommendedAgentTemplate returns the most recent non-deleted row with the flag set, and the fallback asks for exactly one row. Two bounded queries, no page window, no unbounded walk on the create path.

Verified against real MySQL 8.0 rather than only the fake store — TestStore_GetRecommendedAgentTemplate covers nothing-recommended-after-registration, a marked older row beating a newer unmarked one, and a soft-deleted recommendation staying gone. go test ./... is green in CubeOps/.

Not changed: whether RegisterMarketTemplate should persist the flag the Dashboard sends. Doing so as-is would mark every market template recommended, which looks more like a client oversight than an intent to cement — happy to file it separately if you disagree.

[ENCHIGO]

Pushed 85466e6 for the two findings from the last round. One is fixed, one I am deliberately not fixing here — reasoning in the threads, summary:

1. isCMNotFound was too narrow — fixed. The gap defeated the point of this PR: readResponse returned a bare fmt.Errorf for HTTP >= 400, so a not-found reported with an HTTP status could not be classified at all (errors.As(&CMError) fails), and the caller still got the 502 naming defaultAgentTemplateID. The #1327 repro uses the business code, which is why the gap was invisible.

Fixed one level down rather than by string-matching the message: cubemaster now returns a typed *HTTPError{Status, Body} for HTTP >= 400, with a byte-identical message so logs and greps are unaffected, and isCMNotFound accepts either shape. TestIsCMNotFoundCoversBothShapes pins business 130404 / business 404 / HTTP 404 / wrapped HTTP 404, with HTTP 500, business conflict, unrelated and nil as negatives.

wrapCMError still maps *HTTPError to 502 on purpose — teaching it the full mapping changes the status every CubeOps endpoint returns for transport-level failures, which is a larger change than this PR should carry. Left with a comment saying so rather than leaving the asymmetry unexplained.

2. recommended is never persisted — real, but not a one-liner, and not mine to decide. The provenance is already correct in the code (7c40da0 rewrote GetRecommendedAgentTemplate's doc comment to state that neither registration path writes the flag) and the PR description no longer claims otherwise, so nothing here is misleading now.

But persisting req.Recommended in RegisterMarketTemplate would not just fill in a missing column: the Dashboard sends recommended: true on every market registration, so every market template would be flagged, GetRecommendedAgentTemplate (ORDER BY created_at DESC, id DESC) would collapse back to "most recently registered", and the operator's per-template toggle would become meaningless. That is a decision about what recommended means — operator-curated, or set by the act of registering — and it silently changes which template every default create lands on. An API that accepts a field and drops it is still wrong, so it wants its own change with that decision made; happy to open it if a maintainer says which behaviour they want.

go build ./..., go vet, gofmt clean; go test ./internal/service/... ./internal/cubemaster/... ./internal/store/... all pass.

[cubesandboxbot]

The CMError branch keys on ret codes (130404/404), but the HTTPError branch keys only on the HTTP status being exactly 404. A not-found that arrives as a non-404 HTTP status carrying ret_code: 130404 in the body (e.g. HTTP 400 + the 130404 envelope) would slip through here and the actionable 400 in CreateInstance would silently stop firing — the exact "silently stop working" regression this helper exists to prevent. Worth parsing the body's ret_code in HTTPError.IsNotFound() (or in isCMNotFound for the HTTPError case) so both shapes are recognised by content, not by status. Low severity since the #1327 repro (200 body, 130404) and a plain HTTP 404 are both covered.

[cubesandboxbot]

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.