fix(restart): preserve user config volume on default restart (#1822 drift-risk-3) - #1888
Merged
Merged
Conversation
…rift-risk-3)
### Repro
On Canvas: create a workspace named "Hermes Agent" (runtime=langgraph,
model=langgraph default). Open the Config tab, switch the model to a
Minimax provider + Minimax token, hit Save and Restart. The model
reverts to the default on every restart.
### Root cause
`workspace_restart.go` called `findTemplateByName(configsDir, wsName)`
unconditionally when the request body had no explicit `template`:
template := body.Template
if template == "" {
template = findTemplateByName(h.configsDir, wsName)
}
`findTemplateByName` normalises the name ("Hermes Agent" → "hermes-agent")
and ALSO scans every template's `config.yaml` for a matching `name:`
field — a two-layer match that returns non-empty for any workspace whose
name coincides with a template dir OR any template whose config.yaml
claims the same display name.
When the match returned non-empty, the restart handler set
`templatePath = <template>` and the provisioner rewrote the workspace's
config volume from the template on `Start`. The Canvas Save+Restart
flow's `PUT /workspaces/:id/files/config.yaml` had already written the
user's edits to the volume — those got clobbered.
The comment immediately below (line 187) ALREADY said:
// Apply runtime-default template ONLY when explicitly requested
// via "apply_template": true. Use case: runtime was changed via
// Config tab — need new runtime's base files. Normal restarts
// preserve existing config volume (user's model, skills, prompts).
The code contradicted the comment. The design intent was right; the
implementation short-circuited it. Matches drift-risk #3 in #1822's
Docker-vs-EC2 parity tracker ("Config-tab save must flush to DB before
kicking off restart, not deferred").
### Fix
Extracted the template-resolution chain into a pure function
`resolveRestartTemplate(configsDir, wsName, dbRuntime, body)` in a new
`restart_template.go`. Gated the name-based auto-match on
`body.ApplyTemplate`:
1. Explicit `body.Template` → always honoured (caller consent).
2. `ApplyTemplate=true` → name-based auto-match (prior behaviour).
3. `RebuildConfig=true` → org-templates recovery fallback (#239).
4. `ApplyTemplate=true` + dbRuntime → `<runtime>-default/`.
5. Fall through → empty path + "existing-volume" label. Provisioner
reuses the volume. This is the path Canvas Save+Restart now hits.
The handler now calls this helper and uses the returned path directly.
Duplicate rebuild_config blocks at lines 167-186 were consolidated into
the helper's single tier-3 case in passing.
### Abstraction win
`resolveRestartTemplate` is a pure function — no gin context, no DB, no
network. Takes a struct input, returns two strings. The whole priority
chain is unit-testable in a temp dir, which is exactly what
`restart_template_test.go` does.
### Tests
`restart_template_test.go` — 8 table-style unit tests covering every
branch of the priority chain:
- DefaultRestart_PreservesVolume — the regression. Even when a
template's config.yaml `name:` field matches the workspace name
exactly (worst case), a default restart MUST return empty path.
- ExplicitTemplate_AlwaysHonoured — caller-by-name, any mode.
- ApplyTemplate_NameMatch — opt-in restores the auto-match.
- ApplyTemplate_RuntimeDefault — runtime-change flow still works.
- ApplyTemplate_NoMatch_NoRuntime — fallback to existing-volume.
- InvalidExplicitTemplate_ProceedsWithout — traversal attempt stays
inside root, falls through cleanly.
- NonExistentExplicitTemplate — deleted/missing template falls through.
- Priority_ExplicitBeatsApplyTemplate — explicit Template wins over
name-match when both fire.
Full handlers race suite (`go test -race ./internal/handlers/`) still
passes — existing Restart-handler tests unchanged.
### Blast radius
Any restart caller that omitted `apply_template: true` and relied on
name-matching auto-applying a template is now a behaviour change.
Identified call sites in this repo:
- Canvas Save+Restart button (store/canvas.ts) — explicitly the
flow this commit fixes, definitely wanted the fix.
- Canvas Restart button (same file) — same semantics; user expects
a restart, not a template reset.
- Auto-restart sweeper (#1858) — never passes apply_template and
depends on the existing volume having valid config. Separately,
`workspace_provision.go`'s #1858 recovery path detects empty
volumes and auto-applies `<runtime>-default` without going
through findTemplateByName, so recovery is unaffected.
- RestartByID — internal callers; audited, all intended "restart
as-is", none relied on auto-template-match.
No SaaS parity impact — this is a handler behaviour fix that applies
equally to Docker and EC2 backends (both use the same Restart handler
before dispatching to their respective provisioners).
Refs #1822 drift-risk-3.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![molecule-ai[bot]](https://avatars.githubusercontent.com/u/275794187?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Technical Review — PR #1888: fix(restart): preserve user config volume on default restart
APPROVE — correct regression fix, well-tested
What this fixes
Canvas Save+Restart silently reverted user config edits when the workspace name matched a template directory (e.g. "Hermes Agent" → hermes/). The old code unconditionally applied name-based auto-match — even on default restarts — which would overwrite the user's edited config volume with template files.
How it works
New restartTemplateInput struct captures the three restart signals from the request body:
Template— explicit template name, always honouredApplyTemplate— opt-in gate for name-based auto-match (NOT applied by default)RebuildConfig— recovery signal for out-of-band config volume destruction
Priority chain (5 tiers):
- Explicit
Template→ use it ✅ ApplyTemplate=true+ name match → use it ✅RebuildConfig=true→ org-templates recovery fallback ✅ApplyTemplate=true+ known runtime → runtime-default template ✅- Fall through → empty path + "existing-volume" label → preserve user edits ✅
Tier 5 is the critical fix: default Canvas Save+Restart now hits this path and preserves the volume.
Code quality assessment
resolveRestartTemplateis a pure function (no DB, no filesystem writes, no network) — ideal for unit testingrestartTemplateInputtype extraction enables testability without gin contextfindTemplateByName+resolveInsideRootalready exist in codebase — no new primitivesos.Statcheck afterresolveInsideRootis correct (might not be a dir even if path is clean)RebuildConfigtier handles #239 recovery path without affecting normal restart behavior
Test coverage
The regression test TestResolveRestartTemplate_DefaultRestart_PreservesVolume is the right test to have:
- Creates worst-case scenario: template whose
config.yaml name: Hermes Agentmatches workspace name - Asserts
path == ""(no template applied) andlabel == "existing-volume" - Comment explicitly names the regression being locked in
Additional tests cover: explicit template honouring, ApplyTemplate name match, ApplyTemplate runtime default.
Architectural note
The doc comment on restartTemplateInput (ApplyTemplate field) is exemplary — explains the historical regression clearly, names the exact user scenario (Canvas Config tab → Save+Restart), and identifies the original bug. This kind of documentation prevents the same mistake from being re-introduced.
No blockers ✅ — merge when CI is green
HongmingWang-Rabbit
enabled auto-merge
April 23, 2026 21:39
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the Canvas "Save and Restart" regression where editing model / provider / skills / prompts in the Config tab silently reverts on every restart if the workspace's name happens to match a template dir (or any template's
config.yamlname:field). Closes drift-risk #3 from #1822.Repro (before this fix)
Hermes Agent(runtimelanggraph).Any workspace name that normalises to an existing template dir name (
"Hermes" → hermes/,"LangGraph" → langgraph/, etc.) is affected. Scanning templateconfig.yamlname:fields widens the surface further.Root cause
workspace_restart.goranfindTemplateByName(configsDir, wsName)unconditionally when no explicit template was in the body, and the provisioner then rewrote the workspace's config volume from that template onStart— clobbering the user'sPUT /workspaces/:id/files/config.yamlwrite from the Canvas Save flow.The comment at line 187 already stated the correct semantics:
The code contradicted the comment. Design intent was correct; implementation short-circuited it.
Fix
Extracted the template-resolution chain into a pure function
resolveRestartTemplate(configsDir, wsName, dbRuntime, body)in newrestart_template.go. Gated the name-based auto-match onbody.ApplyTemplate:body.Templatenon-emptybody.ApplyTemplate=truefindTemplateByNamebody.RebuildConfig=trueresolveOrgTemplate(#239)body.ApplyTemplate=true+ dbRuntime<runtime>-default/"existing-volume")The Canvas Save+Restart flow now reliably hits tier 5 → volume preserved → user edits survive.
Tests
restart_template_test.go— 8 unit tests covering every branch (including the specific worst-case: a template whoseconfig.yamlname:matches the workspace name exactly). Pure function, no gin / DB / network → tests run in a temp dir.Full
go test -race ./internal/handlers/passes — no regression in existing Restart-handler tests.Test plan
go build ./...— cleango test -race ./internal/handlers/— all existing + 8 new tests passScope notes
apply_templateexplicitly (reset flows) or genuinely want the "restart as-is" semantics this fix enables. The [platform] Provisioner race: auto-restart fails with empty config volume, marks workspace 'failed' (repeat offender today: 2 workspaces) #1858 auto-restart recovery path detects empty volumes via a separate code path and is unaffected.Refs
🤖 Generated with Claude Code