[core-be-agent]
fix(template_import): Remove silent template-dir fallback in ReplaceFiles offline path When the workspace container is offline and writeViaEphemeral fails (docker unavailable), ReplaceFiles previously fell back to writing to the host-side template directory. This silently returned 200 with "source: template" while the file change was invisible after restart because the restart handler reads from the Docker volume, not the template dir (issue #151). Now returns 503 Service Unavailable with a message telling the caller to retry after the workspace starts. The ephemeral write path is the only correct mechanism for offline-container updates. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
7079d4ba01
commit
c9cf240751
@ -267,22 +267,16 @@ func (h *TemplatesHandler) ReplaceFiles(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// Container offline — try ephemeral container to write to volume
|
||||
// Container offline — write to the config volume via ephemeral container.
|
||||
// Do NOT fall back to the host-side template dir: the restart handler
|
||||
// reads from the volume, not the template dir, so a template-dir write
|
||||
// would silently succeed (200) while the file change is invisible to
|
||||
// restarted containers (#151). Surface the error so the caller retries
|
||||
// when Docker is available instead of believing the change persisted.
|
||||
volName := provisioner.ConfigVolumeName(workspaceID)
|
||||
if err := h.writeViaEphemeral(ctx, volName, body.Files); err != nil {
|
||||
// Last resort: write to host-side template dir
|
||||
destDir := h.resolveTemplateDir(wsName)
|
||||
if destDir == "" {
|
||||
log.Printf("ReplaceFiles: writeViaEphemeral failed and no template dir for %s: %v", workspaceID, err)
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to write files to workspace"})
|
||||
return
|
||||
}
|
||||
os.MkdirAll(destDir, 0o755)
|
||||
if err := writeFiles(destDir, body.Files); err != nil {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request body"})
|
||||
return
|
||||
}
|
||||
c.JSON(http.StatusOK, gin.H{"status": "replaced", "workspace": workspaceID, "files": len(body.Files), "source": "template"})
|
||||
log.Printf("ReplaceFiles: writeViaEphemeral failed for %s (workspace %s offline): %v", wsName, workspaceID, err)
|
||||
c.JSON(http.StatusServiceUnavailable, gin.H{"error": "workspace offline — retry after it starts"})
|
||||
return
|
||||
}
|
||||
|
||||
|
||||
@ -452,3 +452,43 @@ func TestReplaceFiles_PathTraversal(t *testing.T) {
|
||||
t.Errorf("unmet sqlmock expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestReplaceFiles_OfflineDocker_Returns503 ensures that when the workspace
|
||||
// container is offline AND the ephemeral Docker write fails (docker unavailable),
|
||||
// ReplaceFiles returns 503 instead of silently falling back to the template
|
||||
// directory. A template-dir write would be invisible after restart because
|
||||
// the restart handler reads from the Docker volume, not the template dir (#151).
|
||||
func TestReplaceFiles_OfflineDocker_Returns503(t *testing.T) {
|
||||
mock := setupTestDB(t)
|
||||
setupTestRedis(t)
|
||||
|
||||
// nil docker → TemplatesHandler.docker == nil → findContainer returns "" (offline)
|
||||
// → writeViaEphemeral returns error (docker not available) → handler should
|
||||
// return 503, NOT fall back to the host-side template dir.
|
||||
handler := NewTemplatesHandler(t.TempDir(), nil, nil)
|
||||
|
||||
mock.ExpectQuery(`SELECT name, COALESCE\(instance_id, ''\), COALESCE\(runtime, ''\) FROM workspaces WHERE id =`).
|
||||
WithArgs("ws-offline").
|
||||
WillReturnRows(sqlmock.NewRows([]string{"name", "instance_id", "runtime"}).AddRow("Offline Agent", "", ""))
|
||||
|
||||
body := `{"files": {"initial-prompt.md": "updated prompt"}}`
|
||||
w := httptest.NewRecorder()
|
||||
c, _ := gin.CreateTestContext(w)
|
||||
c.Params = gin.Params{{Key: "id", Value: "ws-offline"}}
|
||||
c.Request = httptest.NewRequest("PUT", "/workspaces/ws-offline/files", bytes.NewBufferString(body))
|
||||
c.Request.Header.Set("Content-Type", "application/json")
|
||||
|
||||
handler.ReplaceFiles(c)
|
||||
|
||||
if w.Code != http.StatusServiceUnavailable {
|
||||
t.Errorf("expected 503, got %d: %s", w.Code, w.Body.String())
|
||||
}
|
||||
|
||||
if !strings.Contains(w.Body.String(), "offline") {
|
||||
t.Errorf("response should mention 'offline': %s", w.Body.String())
|
||||
}
|
||||
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unmet sqlmock expectations: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user