feat(controller): retry failed golden builds for Agents and SandboxTemplates by danielle-mustillo · Pull Request #3335 · kagent-dev/kagent

danielle-mustillo · 2026-10-23T18:59:10Z

Why

A golden build can fail for an infrastructure reason, such as a lost worker pod or a full node disk, or for a config fault. Substrate v0.7.0-alpha1 treats every golden error as terminal: once it records error_message, it never works on that template again. The Agent or SandboxTemplate stays not Ready until an operator deletes and reapplies it.

What changed

On any golden error, the Agent and SandboxTemplate reconcilers delete the failed ActorTemplate by UID and recreate it after 15s, 1m, 4m, then every 10m, with up to 20% jitter and no limit. A ready build resets the count.
While a retry waits, Ready=False has the new reason ActorTemplateRetrying, e.g. golden build failed (GoldenActorCrashed), retry 1 at <time>. Status carries only the reason; the full Substrate error goes to the controller log, because GoldenActorInvalid can quote configuration. ActorTemplateFailed is no longer produced.
4 new runtime_revision columns hold the retry count, failed UID, retry time and the ActorTemplate spec digest, so a controller restart keeps the schedule. GC deletes a rebuild the controller never recorded only when its digest matches.

Tradeoffs

No retry limit. A limit makes any outage longer than the backoff terminal again. A config fault retries every 10m until the user edits the resource, and the new revision starts from zero.
Retrying in Substrate instead (ateapi: retry a crashed golden actor up to 3 times substrate#98, Retry golden builds after worker loss and infra Run errors substrate#99) needs a Substrate release. This works with the version kagent pins today.

Blast Radius

The delete has a UID precondition, so it cannot remove a template that was already replaced.
The columns are added to 000001_initial.sql in place, so an existing database must be reinstalled.
No UI change. The Agents page shows a retrying Agent as failed, with the retry in its message.

Size

Against main a9a53207:

Part	Files	Added	Removed
Source	13	371	125
Tests	14	862	102
Docs	2	25	6
Total	29	1258	233

Verification

go test for internal/controller, internal/database, internal/substrate and pkg/migrations in go/core, with PostgreSQL and envtest, passes.
Kind at 3285acf5, Agent and SandboxTemplate with a crashed golden build: retry 1, a new template UID, then Ready. Substrate's raw error text appears in status 0 times. The key-free suite passes 37 of 37.
Kind, Agent whose image digest was pushed only after retry 3: Ready on retry 4 with no manual step.

AI use: AI-assisted, I reviewed it.

moezdil · 2026-10-23T23:29:21Z

    actor_template_uid       TEXT        NOT NULL DEFAULT '',
+    -- Golden-build retries started after GoldenActorCrashed, the crashed
+    -- ActorTemplate UID the latest retry answers, and when it recreates.
+    golden_attempts          INT         NOT NULL DEFAULT 0 CHECK (golden_attempts >= 0),


could a new migration add these for existing dbs?

We will start to have immutable DBs with RCs later this week/early next week

EItanya

🤖 AI-generated review.

Can we contain more of this behavior within goldenRetries? Both reconcilers, their observations, status builders, and pollers currently need to understand golden retries. Having the recovery component return a preparation status and next-check deadline would keep those callers simpler and give us one place to configure the policy or remove it when Substrate handles recovery.

EItanya · 2026-10-27T14:52:36Z

 	// but database finalization fails, the durable deletion marker keeps this
 	// revision discoverable so the next sweep can safely retry the sequence.
-	if err := r.templates.DeleteActorTemplate(collectionCtx, revision.ActorTemplateAtespace, revision.ActorTemplateName); err != nil {
+	if err == nil && (revision.ActorTemplateUID == "" || template.GetMetadata().GetUid() != revision.ActorTemplateUID) && !revision.RecordedTemplateCrashed() {


Can we verify that a replacement belongs to this revision before allowing GC to delete it? RecordedTemplateCrashed() only tells us that the old UID crashed. If that template is replaced under the same name with different immutable inputs, this skips the UID mismatch check and deletes the replacement using its own UID. The normal reconciler would reject that as a conflict. We should keep that protection when collecting an unrecorded rebuild.

Done, implemented a digest check

Done, digest check in cc1cdcf.

EItanya · 2026-10-27T14:52:36Z

+	for _, pinned := range []string{
+		`reasonGoldenActorCrashed\s*=\s*"GoldenActorCrashed"\n`,
+		`r\.fail\(ctx, tmpl, reasonGoldenActorCrashed, "golden actor crashed before its snapshot was taken"\)`,
+		`ErrorMessage = truncateUTF8\(reason\+": "\+msg, maxGoldenErrorMessageLen\)`,


Optional: can we avoid asserting the exact Substrate source here? Production only depends on the GoldenActorCrashed: prefix, but these regexes also pin the private helper calls and full diagnostic text. A harmless upstream refactor or wording change would fail the test without changing the contract we consume. I'd keep the classification tests and drop the checks on those implementation details.

danielle-mustillo · 2026-10-27T19:36:43Z

🤖 AI-generated review.

Can we contain more of this behavior within goldenRetries? Both reconcilers, their observations, status builders, and pollers currently need to understand golden retries. Having the recovery component return a preparation status and next-check deadline would keep those callers simpler and give us one place to configure the policy or remove it when Substrate handles recovery.

Done.

As a bonus found when I was implementing the review comments:

Every golden error now retries, not only GoldenActorCrashed. This was missed first pass.
Status shows only the failure reason, and the full Substrate error goes to the controller log, because some substrate errors can quote the whole configuration (bad UX)

danielle-mustillo · 2026-10-27T19:42:45Z

Can we contain more of this behavior within goldenRetries?

Done.

EItanya · 2026-10-27T20:27:59Z

+func resetGoldenRetry(ctx context.Context, tx pgx.Tx, revision string) error {
+	return execSQL(ctx, tx, `
+		UPDATE runtime_revision SET golden_attempts = 0, golden_failure_uid = '', golden_retry_at = NULL, updated_at = NOW()
+		WHERE revision = $1 AND golden_attempts > 0
+	`, revision)
+}
+
+type GoldenRetry struct {
+	Retries int
+	RetryAt time.Time
+}
+
+func (c *Client) GoldenRetryAttempts(ctx context.Context, revision string) (int, error) {
+	attempts, err := queryOne(ctx, c.db, `
+		SELECT golden_attempts FROM runtime_revision WHERE revision = $1
+	`, pgx.RowTo[int], revision)
+	if err != nil {
+		return 0, fmt.Errorf("get golden retry attempts for runtime revision %s: %w", revision, notFoundOr(err))
+	}
+	return attempts, nil
+}
+
+func (c *Client) EnsureGoldenRetry(ctx context.Context, revision, failedTemplateUID string, now time.Time, delays []time.Duration) (GoldenRetry, error) {


Can we please get comments on these functions so it's easy to grok what they should do

EItanya · 2026-10-27T20:33:17Z

+func (g goldenRetries) observe(ctx context.Context, store goldenRetryStore, templates actorTemplateClient, revision string,
+	template *ateapipb.ActorTemplate, record func(database.RecordRevisionOptions) error) (goldenProgress, error) {
+	golden := template.GetStatus().GetGoldenSnapshotStatus()
+	if err := record(database.RecordRevisionOptions{Ready: golden.GetGoldenTag() != nil}); err != nil {


🤖 AI-generated follow-up.

Can we combine recording the revision and updating/reading its retry state into one store transaction? observe currently calls record(...), then EnsureGoldenRetry opens another transaction on the same row, or GoldenRetryAttempts does a separate read. There are no API calls between those database operations, so the record methods could handle them atomically and return the persisted retry state. That would simplify the persistence sequencing currently hidden behind the callback.

DeleteActorTemplate should stay after the transaction commits, so we don't hold database locks across the Substrate call.

moezdil · 2026-10-29T06:06:56Z

    actor_template_name      TEXT        NOT NULL,
    actor_template_uid       TEXT        NOT NULL DEFAULT '',
+    actor_template_spec_digest TEXT      NOT NULL DEFAULT '',
+    golden_retries           INT         NOT NULL DEFAULT 0 CHECK (golden_retries >= 0),


migration-immutability check fails on this edit

When Substrate fails an ActorTemplate with GoldenActorCrashed, the Agent controller now deletes the crashed template by UID after a backoff and recreates it under the same name, up to 3 retries (15s, 1m, 4m plus up to 20% jitter). The runtime_revision row keeps the retry count, the crashed template UID and the retry time, so passes and controller restarts do not spend the budget twice and a new revision starts fresh. While waiting, Ready is False with reason ActorTemplateRetrying; after the budget, reason ActorTemplateFailed names the build count and Substrate's message. GoldenActorInvalid, GoldenTagConflict and other golden errors stay terminal. Signed-off-by: Danielle Mustillo <danielle.mustillo@solo.io>

…ackoff A golden build that crashes during an infrastructure outage longer than the old three-retry budget left the Agent failed until someone edited it. The controller now retries forever: 15s, 1m, 4m, then every 10m, each with up to 20% jitter, and reports every crash as Ready=False ActorTemplateRetrying. ActorTemplateFailed keeps meaning a terminal golden error that nothing retries. - Move the retry, its timers and stop into goldenRetries, one per queue, and drop the in-memory schedule: runtime_revision keeps the retry time, so a restarted controller re-arms the retry without spending another attempt. - Reset the stored attempts when the revision's golden build succeeds. - Keep ActorTemplateRetrying on any template delete error. - Preparing names the retry while the crashed build rebuilds. - Runtime revision GC collects a rebuild that was never recorded by deleting it by UID when the recorded UID is the crashed one. - Import ateapipb from the Substrate v0.7 path. Signed-off-by: Danielle Mustillo <danielle.mustillo@solo.io>

A SandboxTemplate whose golden build crashed stayed not Ready until someone deleted and re-applied it. The sandbox reconciler now records the revision before it checks the golden status, and hands a GoldenActorCrashed template to its own goldenRetries, built beside the preparation queue. Ready shows ActorTemplateRetrying with the retry number while a retry waits, and Preparing names the retry while the rebuild is pending. The poller skips a waiting retry, so its timer or the queue's backoff wakes it instead of a read every second. RecordSandboxRevision resets the stored golden retries when a sandbox revision becomes ready, like RecordRuntimeRevision. Other golden errors keep today's retryable PreparationFailed. docs/architecture/sandboxes.md describes the retries. Signed-off-by: Danielle Mustillo <danielle.mustillo@solo.io>

Build the agent runtime observation in one place: observePreparation now takes the golden retry count. Flatten the sandbox pollPending condition into early continues. Name the GC rebuild check RuntimeArtifact.RecordedTemplateCrashed, rename ScheduleGoldenRetry to EnsureGoldenRetry since repeat calls reuse the stored schedule, and rename GoldenRetry.Attempts to Retries. Remove comments that restated the code. Signed-off-by: Danielle Mustillo <danielle.mustillo@solo.io>

Drop the per-key timers in goldenRetries. A waiting retry now carries its RetryAt on the ReconciliationFailure, and the 1-second pending poll requeues it once that time passes, so the schedule needs no in-memory state or stop hook. Fold DeleteActorTemplateUID into DeleteActorTemplate, which now takes the UID, and use it from both the golden retry and runtime revision GC. Signed-off-by: Danielle Mustillo <danielle.mustillo@solo.io>

Move the poll decision into needsPreparationPoll so it returns a value instead of steering the loop with continue. A break there would have skipped every sandbox after one waiting golden retry, and no test could catch it reliably because KRT lists states in map order. Table-test the decision: a waiting retry is skipped, while a due retry, a missing finalizer and a deletion during a waiting retry are requeued. Signed-off-by: Danielle Mustillo <danielle.mustillo@solo.io>

Contain golden preparation in goldenRetries. observe records the revision first, through a callback that each reconciler supplies, so a retry always finds its recorded row and Ready follows persistence. It then returns one goldenProgress outcome: building (with the retry count), ready, retrying (failure and next check) or failed (the terminal golden error). Reconcilers, pollers and status builders read that outcome and no longer inspect the ActorTemplate's golden snapshot status. The outcome replaces GoldenRetries, ReconciliationFailure.RetryAt and the separate GoldenRetryAttempts reads, and the sandbox no longer keeps the observed template. Agents and sandboxes publish the outcome the same way. Both pollers wake on goldenProgress.due; the sandbox poller also wakes for a template that has not been observed yet, and other retryable preparation errors stay on the work queue's backoff. The sandbox reconciler publishes goldenProgress.failure as is, so a golden failure other than a crash reports a terminal ActorTemplateFailed with Substrate's message, as for Agents. Keep GC conflict protection for unrecorded golden rebuilds. Each runtime revision records a digest of the ActorTemplate's immutable inputs. After a golden crash, GC collects a template whose UID was never recorded only when its digest matches, so another template under the same name is refused like the reconciler's conflict check. Classify golden crashes without pinning Substrate's source, and drop comments that restate the code. Signed-off-by: Danielle Mustillo <danielle.mustillo@solo.io>

Substrate stops working on a template once it writes error_message, and each rebuild gets a new UID, so the golden actor, tag and egress policy are fresh. Delete and recreate is the only recovery for every reason, so drop the crash-only filter and the terminal ActorTemplateFailed phase, and say so in the sandbox architecture doc. Signed-off-by: Danielle Mustillo <danielle.mustillo@solo.io>

… naming Substrate formats a golden error as "<Reason>: <message>", and for GoldenActorInvalid and GoldenEgressPolicyConflict the message is a backend error that can quote configuration. Status now carries only the reason, or "unknown" when the message has no reason prefix, and the controller logs the full message. This matches the architecture doc's promise that raw backend messages stay out of Kubernetes status. Every golden error is retried now, so the names that said "crash" (the golden_crash_uid column, GoldenCrashUID, RecordedTemplateCrashed, the status message and the test helpers) say "failure" instead. The docs describe golden failures as retried with reason ActorTemplateRetrying rather than terminal. Agents checked an immutable ActorTemplate conflict twice: once in reconcileAgent and again in the KRT transform, which needed the observed template carried through the observation and a protobuf-aware Equals. reconcileAgent now publishes ActorTemplateConflict itself, the same way it publishes ActorTemplateRejected and the way sandboxes already work, so the observed template, its Equals handling and the transform branch are gone. The constructors now set the golden retry schedule. A zero-value goldenRetries no longer falls back to the defaults, so an empty schedule surfaces as an error instead of being replaced silently. GetRuntimeRevision now reads the ActorTemplate spec digest, so the Agent read model no longer returns an empty field. Signed-off-by: Danielle Mustillo <danielle.mustillo@solo.io>

Signed-off-by: Danielle Mustillo <danielle.mustillo@solo.io>

…etry The store saved golden_retry_at, so it needed the backoff delays and a random jitter that had to be computed once. It now saves golden_failed_at instead. RecordRuntimeRevision and RecordSandboxRevision update the retry state in the same transaction as the revision and return it, which removes EnsureGoldenRetry, GoldenRetryAttempts and the record callback in observe. The controller computes the retry time from the failure time, the delays and a jitter taken from a hash of the failed template's UID, so every reconcile and restart gets the same time. The golden failure is logged once, when the store first records it, instead of on every reconcile. Signed-off-by: Danielle Mustillo <danielle.mustillo@solo.io>

- GC skips the ActorTemplate delete when the template is already gone. An empty UID waives Substrate's delete precondition, so the delete could remove a template recreated under the same name. The read result is classified in collectableTemplate. - The sandbox poll no longer requeues failed preparations every second; they stay on the work queue's backoff. - Fold goldenProgress.waiting into due, its only caller. - Rename golden_attempts to golden_retries to match GoldenRetry.Retries. Signed-off-by: Danielle Mustillo <danielle.mustillo@solo.io>

Signed-off-by: Danielle Mustillo <danielle.mustillo@solo.io>

…onciler test Signed-off-by: Danielle Mustillo <danielle.mustillo@solo.io>

This was referenced Oct 23, 2026

ateapi: retry a crashed golden actor up to 3 times kagent-dev/substrate#98

Closed

Retry golden builds after worker loss and infra Run errors kagent-dev/substrate#99

Closed

moezdil reviewed Oct 23, 2026

View reviewed changes

danielle-mustillo force-pushed the danielle-mustillo/552-c1 branch from 65a0c35 to 17249ed Compare October 26, 2026 16:03

danielle-mustillo changed the title ~~Retry crashed golden builds within a durable budget~~ feat(controller): retry crashed golden builds within a durable budget Oct 26, 2026

github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Oct 26, 2026

danielle-mustillo force-pushed the danielle-mustillo/552-c1 branch 2 times, most recently from 9020188 to 2174e1f Compare October 26, 2026 23:06

danielle-mustillo changed the title ~~feat(controller): retry crashed golden builds within a durable budget~~ feat(controller): retry crashed golden builds for Agents and SandboxTemplates Oct 26, 2026

github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Oct 26, 2026

danielle-mustillo marked this pull request as ready for review October 27, 2026 05:41

danielle-mustillo requested a review from a team as a code owner October 27, 2026 05:41

EItanya reviewed Oct 27, 2026

View reviewed changes

danielle-mustillo changed the title ~~feat(controller): retry crashed golden builds for Agents and SandboxTemplates~~ feat(controller): retry failed golden builds for Agents and SandboxTemplates Oct 27, 2026

github-actions Bot added enhancement New feature or request and removed enhancement New feature or request labels Oct 27, 2026

danielle-mustillo requested a review from EItanya October 27, 2026 19:43

EItanya reviewed Oct 27, 2026

View reviewed changes

danielle-mustillo force-pushed the danielle-mustillo/552-c1 branch from 73f8061 to 0b25738 Compare October 27, 2026 22:50

moezdil reviewed Oct 29, 2026

View reviewed changes

EItanya previously approved these changes Oct 29, 2026

View reviewed changes

EItanya added this pull request to the merge queue Oct 29, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 29, 2026

danielle-mustillo added 14 commits October 29, 2026 12:38

docs(controller): document golden build retry functions

d8bd757

Signed-off-by: Danielle Mustillo <danielle.mustillo@solo.io>

test(database): unwrap golden retry in actor JWT grant test

47d20c9

Signed-off-by: Danielle Mustillo <danielle.mustillo@solo.io>

test(controller): read golden retry from RecordRuntimeRevision in rec…

e22795d

…onciler test Signed-off-by: Danielle Mustillo <danielle.mustillo@solo.io>

danielle-mustillo dismissed EItanya’s stale review via e22795d October 29, 2026 16:41

danielle-mustillo force-pushed the danielle-mustillo/552-c1 branch from 0b25738 to e22795d Compare October 29, 2026 16:41

EItanya approved these changes Oct 29, 2026

View reviewed changes

EItanya enabled auto-merge October 29, 2026 16:54

EItanya added this pull request to the merge queue Oct 29, 2026

Merged via the queue into kagent-dev:main with commit 3f62d6a Oct 29, 2026
30 of 55 checks passed

Conversation

danielle-mustillo commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What changed

Tradeoffs

Blast Radius

Size

Verification

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EItanya left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielle-mustillo commented Oct 27, 2026

Uh oh!

danielle-mustillo commented Oct 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielle-mustillo Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

danielle-mustillo commented Oct 23, 2026 •

edited

Loading

danielle-mustillo Oct 27, 2026 •

edited

Loading