Skip to content

Commit 9b084a5

Browse files
committed
fix(upgrade): persist apply failures and support new release asset schema
- record upgrade helper failures to a status file and log file - load persisted apply errors on server startup and surface them in the UI - temporarily allow assets without sha256 metadata during download
1 parent 625606a commit 9b084a5

15 files changed

Lines changed: 455 additions & 86 deletions

File tree

cli/app_test.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1891,12 +1891,11 @@ func TestExecuteUpgradeCheckPrintsTable(t *testing.T) {
18911891
t.Fatalf("url = %q, want latest release endpoint", req.URL.String())
18921892
}
18931893
return jsonResponse(http.StatusOK, fmt.Sprintf(`{
1894-
"version":"v0.2.7",
1895-
"download_base_url":"https://downloads.example.test/csgclaw/v0.2.7",
1894+
"name":"v0.2.7",
18961895
"assets":[
1897-
{"name":"%s"}
1896+
{"name":"%s","browser_download_url":"https://downloads.example.test/csgclaw/v0.2.7/%s"}
18981897
]
1899-
}`, assetName)), nil
1898+
}`, assetName, assetName)), nil
19001899
}),
19011900
}
19021901

@@ -1925,7 +1924,7 @@ func TestExecuteUpgradeDefaultsToCheckResult(t *testing.T) {
19251924
stdout: &stdout,
19261925
stderr: &bytes.Buffer{},
19271926
httpClient: roundTripFunc(func(req *http.Request) (*http.Response, error) {
1928-
return jsonResponse(http.StatusOK, `{"version":"v0.2.7","assets":[]}`), nil
1927+
return jsonResponse(http.StatusOK, `{"name":"v0.2.7","assets":[]}`), nil
19291928
}),
19301929
}
19311930

@@ -1956,11 +1955,11 @@ func TestExecuteUpgradeSupportsJSONOutput(t *testing.T) {
19561955
stderr: &bytes.Buffer{},
19571956
httpClient: roundTripFunc(func(req *http.Request) (*http.Response, error) {
19581957
return jsonResponse(http.StatusOK, fmt.Sprintf(`{
1959-
"version":"v0.2.7",
1958+
"name":"v0.2.7",
19601959
"assets":[
1961-
{"name":"%s"}
1960+
{"name":"%s","browser_download_url":"https://downloads.example.test/csgclaw/v0.2.7/%s"}
19621961
]
1963-
}`, assetName)), nil
1962+
}`, assetName, assetName)), nil
19641963
}),
19651964
}
19661965

cli/serve/serve.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,11 @@ func startServerWithConfigPath(ctx context.Context, run *command.Context, cfg co
477477
})
478478
},
479479
})
480+
if message, err := upgrade.ConsumeApplyFailure(configPath); err != nil {
481+
slog.Warn("load upgrade helper failure", "error", err)
482+
} else if message != "" {
483+
upgradeManager.MarkUpgradeFailed(errors.New(message))
484+
}
480485
return RunServer(server.Options{
481486
ListenAddr: cfg.Server.ListenAddr,
482487
Service: svc,

cli/serve/serve_test.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package serve
33
import (
44
"bytes"
55
"context"
6+
"errors"
67
"fmt"
78
"log/slog"
89
"os"
@@ -22,6 +23,7 @@ import (
2223
internalonboard "csgclaw/internal/onboard"
2324
agentruntime "csgclaw/internal/runtime"
2425
"csgclaw/internal/server"
26+
"csgclaw/internal/upgrade"
2527
)
2628

2729
func TestServeForegroundPreflightsCSGHubLiteProvider(t *testing.T) {
@@ -644,6 +646,57 @@ func TestServeForegroundPassesContextToServer(t *testing.T) {
644646
}
645647
}
646648

649+
func TestStartServerWithConfigPathLoadsPersistedUpgradeFailure(t *testing.T) {
650+
restore := stubServeDependencies(t)
651+
defer restore()
652+
653+
origRunServer := RunServer
654+
t.Cleanup(func() {
655+
RunServer = origRunServer
656+
})
657+
658+
configPath := filepath.Join(t.TempDir(), "config.toml")
659+
artifacts, err := upgrade.ResolveApplyArtifacts(configPath)
660+
if err != nil {
661+
t.Fatalf("ResolveApplyArtifacts() error = %v", err)
662+
}
663+
if err := artifacts.RecordFailure(errors.New("restart daemon: boom")); err != nil {
664+
t.Fatalf("RecordFailure() error = %v", err)
665+
}
666+
667+
RunServer = func(opts server.Options) error {
668+
if opts.Upgrade == nil {
669+
return errors.New("Upgrade = nil, want configured manager")
670+
}
671+
status := opts.Upgrade.Status()
672+
if !strings.Contains(status.LastError, "restart daemon: boom") {
673+
return fmt.Errorf("LastError = %q, want persisted failure", status.LastError)
674+
}
675+
if !strings.Contains(status.LastError, artifacts.LogPath) {
676+
return fmt.Errorf("LastError = %q, want log path", status.LastError)
677+
}
678+
return nil
679+
}
680+
681+
run := testContext()
682+
cfg := config.Config{
683+
Server: config.ServerConfig{
684+
ListenAddr: "127.0.0.1:18080",
685+
AccessToken: "pc-secret",
686+
},
687+
Sandbox: config.SandboxConfig{
688+
Provider: config.DefaultSandboxProvider,
689+
},
690+
}
691+
692+
if err := startServerWithConfigPath(context.Background(), run, cfg, nil, nil, nil, nil, nil, configPath, "table"); err != nil {
693+
t.Fatalf("startServerWithConfigPath() error = %v", err)
694+
}
695+
if _, err := os.Stat(artifacts.StatusPath); !errors.Is(err, os.ErrNotExist) {
696+
t.Fatalf("status file still exists after startup; stat err = %v", err)
697+
}
698+
}
699+
647700
func TestConfigureServeLoggerRejectsUnsupportedLevel(t *testing.T) {
648701
_, err := configureServeLogger(&bytes.Buffer{}, "trace")
649702
if err == nil {

cli/upgrade/upgrade.go

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,38 +50,50 @@ func (c cmd) Run(ctx context.Context, run *command.Context, args []string, globa
5050
return fmt.Errorf("upgrade does not accept positional arguments")
5151
}
5252

53+
applyArtifacts := upgrade.ApplyArtifactsFromEnv()
54+
fail := func(err error) error {
55+
err = explainError(run.Program, err)
56+
if recordErr := applyArtifacts.RecordFailure(err); recordErr != nil {
57+
return fmt.Errorf("%w\nAlso failed to record upgrade helper status: %v", err, recordErr)
58+
}
59+
return err
60+
}
61+
5362
client := newUpgradeClient(run)
5463
result, err := client.Check(ctx, appversion.Current())
5564
if err != nil {
56-
return explainError(run.Program, err)
65+
return fail(err)
5766
}
5867
if *checkOnly || !result.UpdateAvailable {
68+
_ = applyArtifacts.ClearStatus()
5969
return renderResult(globals.Output, run.Stdout, result)
6070
}
6171
if result.Asset == nil {
62-
return fmt.Errorf("matched release asset is required for installation")
72+
return fail(fmt.Errorf("matched release asset is required for installation"))
6373
}
6474

6575
prepared, err := client.PrepareRelease(ctx, *result.Asset, "")
6676
if err != nil {
67-
return explainError(run.Program, err)
77+
return fail(err)
6878
}
6979
defer os.RemoveAll(prepared.WorkDir)
7080

7181
installed, err := client.InstallPrepared(prepared)
7282
if err != nil {
73-
return explainError(run.Program, err)
83+
return fail(err)
7484
}
7585
if *noRestart {
86+
_ = applyArtifacts.ClearStatus()
7687
return renderInstallResult(globals.Output, run.Stdout, result, installed, upgrade.RestartResult{}, run.Program, true)
7788
}
7889

7990
restarted, err := client.RestartIfRunning(ctx, installed, upgrade.RestartOptions{
8091
ConfigPath: globals.Config,
8192
})
8293
if err != nil {
83-
return explainError(run.Program, err)
94+
return fail(err)
8495
}
96+
_ = applyArtifacts.ClearStatus()
8597
return renderInstallResult(globals.Output, run.Stdout, result, installed, restarted, run.Program, false)
8698
}
8799

cli/upgrade/upgrade_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ func TestRunNoRestartInstallsBundle(t *testing.T) {
4141
switch req.URL.String() {
4242
case "https://example.test/releases/latest":
4343
return jsonResponse(http.StatusOK, `{
44-
"version":"v0.2.7",
45-
"assets":[{"name":"csgclaw_v0.2.7_darwin_arm64.tar.gz","download_url":"https://downloads.example.test/csgclaw.tar.gz","size":`+strconv.Itoa(len(archive))+`,"sha256":"`+hex.EncodeToString(sum[:])+`"}]
44+
"name":"v0.2.7",
45+
"assets":[{"name":"csgclaw_v0.2.7_darwin_arm64.tar.gz","browser_download_url":"https://downloads.example.test/csgclaw.tar.gz","size":`+strconv.Itoa(len(archive))+`,"sha256":"`+hex.EncodeToString(sum[:])+`"}]
4646
}`), nil
4747
case "https://downloads.example.test/csgclaw.tar.gz":
4848
return &http.Response{
@@ -120,8 +120,8 @@ func TestRunRestartsRunningDaemonAfterInstall(t *testing.T) {
120120
switch req.URL.String() {
121121
case "https://example.test/releases/latest":
122122
return jsonResponse(http.StatusOK, `{
123-
"version":"v0.2.7",
124-
"assets":[{"name":"csgclaw_v0.2.7_darwin_arm64.tar.gz","download_url":"https://downloads.example.test/csgclaw.tar.gz","size":`+strconv.Itoa(len(archive))+`,"sha256":"`+hex.EncodeToString(sum[:])+`"}]
123+
"name":"v0.2.7",
124+
"assets":[{"name":"csgclaw_v0.2.7_darwin_arm64.tar.gz","browser_download_url":"https://downloads.example.test/csgclaw.tar.gz","size":`+strconv.Itoa(len(archive))+`,"sha256":"`+hex.EncodeToString(sum[:])+`"}]
125125
}`), nil
126126
case "https://downloads.example.test/csgclaw.tar.gz":
127127
return &http.Response{
@@ -214,8 +214,8 @@ func TestRunInstallErrorExplainsBundleRequirement(t *testing.T) {
214214
switch req.URL.String() {
215215
case "https://example.test/releases/latest":
216216
return jsonResponse(http.StatusOK, `{
217-
"version":"v0.2.7",
218-
"assets":[{"name":"csgclaw_v0.2.7_darwin_arm64.tar.gz","download_url":"https://downloads.example.test/csgclaw.tar.gz","size":`+strconv.Itoa(len(archive))+`,"sha256":"`+hex.EncodeToString(sum[:])+`"}]
217+
"name":"v0.2.7",
218+
"assets":[{"name":"csgclaw_v0.2.7_darwin_arm64.tar.gz","browser_download_url":"https://downloads.example.test/csgclaw.tar.gz","size":`+strconv.Itoa(len(archive))+`,"sha256":"`+hex.EncodeToString(sum[:])+`"}]
219219
}`), nil
220220
case "https://downloads.example.test/csgclaw.tar.gz":
221221
return &http.Response{
@@ -288,8 +288,8 @@ func TestRunRestartErrorExplainsManualRecovery(t *testing.T) {
288288
switch req.URL.String() {
289289
case "https://example.test/releases/latest":
290290
return jsonResponse(http.StatusOK, `{
291-
"version":"v0.2.7",
292-
"assets":[{"name":"csgclaw_v0.2.7_darwin_arm64.tar.gz","download_url":"https://downloads.example.test/csgclaw.tar.gz","size":`+strconv.Itoa(len(archive))+`,"sha256":"`+hex.EncodeToString(sum[:])+`"}]
291+
"name":"v0.2.7",
292+
"assets":[{"name":"csgclaw_v0.2.7_darwin_arm64.tar.gz","browser_download_url":"https://downloads.example.test/csgclaw.tar.gz","size":`+strconv.Itoa(len(archive))+`,"sha256":"`+hex.EncodeToString(sum[:])+`"}]
293293
}`), nil
294294
case "https://downloads.example.test/csgclaw.tar.gz":
295295
return &http.Response{

internal/upgrade/apply_state.go

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
package upgrade
2+
3+
import (
4+
"encoding/json"
5+
"errors"
6+
"fmt"
7+
"os"
8+
"path/filepath"
9+
"strings"
10+
"time"
11+
12+
"csgclaw/internal/config"
13+
)
14+
15+
const (
16+
applyStatusEnvVar = "CSGCLAW_UPGRADE_STATUS_PATH"
17+
applyLogEnvVar = "CSGCLAW_UPGRADE_LOG_PATH"
18+
applyStatusFileName = "upgrade-helper-status.json"
19+
applyLogFileName = "upgrade-helper.log"
20+
applyLogsDirName = "logs"
21+
)
22+
23+
type ApplyArtifacts struct {
24+
StatusPath string
25+
LogPath string
26+
}
27+
28+
type applyFailureRecord struct {
29+
Message string `json:"message"`
30+
LogPath string `json:"log_path,omitempty"`
31+
UpdatedAt time.Time `json:"updated_at"`
32+
}
33+
34+
func ResolveApplyArtifacts(configPath string) (ApplyArtifacts, error) {
35+
dir, err := applyArtifactsDir(configPath)
36+
if err != nil {
37+
return ApplyArtifacts{}, err
38+
}
39+
return ApplyArtifacts{
40+
StatusPath: filepath.Join(dir, applyLogsDirName, applyStatusFileName),
41+
LogPath: filepath.Join(dir, applyLogsDirName, applyLogFileName),
42+
}, nil
43+
}
44+
45+
func PrepareApplyArtifacts(configPath string) (ApplyArtifacts, error) {
46+
artifacts, err := ResolveApplyArtifacts(configPath)
47+
if err != nil {
48+
return ApplyArtifacts{}, err
49+
}
50+
if err := os.MkdirAll(filepath.Dir(artifacts.StatusPath), 0o755); err != nil {
51+
return ApplyArtifacts{}, fmt.Errorf("create upgrade helper state dir: %w", err)
52+
}
53+
if err := os.Remove(artifacts.StatusPath); err != nil && !errors.Is(err, os.ErrNotExist) {
54+
return ApplyArtifacts{}, fmt.Errorf("remove stale upgrade helper status: %w", err)
55+
}
56+
return artifacts, nil
57+
}
58+
59+
func ApplyArtifactsFromEnv() ApplyArtifacts {
60+
return ApplyArtifacts{
61+
StatusPath: strings.TrimSpace(os.Getenv(applyStatusEnvVar)),
62+
LogPath: strings.TrimSpace(os.Getenv(applyLogEnvVar)),
63+
}
64+
}
65+
66+
func (a ApplyArtifacts) Enabled() bool {
67+
return strings.TrimSpace(a.StatusPath) != ""
68+
}
69+
70+
func (a ApplyArtifacts) Env() []string {
71+
if !a.Enabled() {
72+
return nil
73+
}
74+
env := []string{fmt.Sprintf("%s=%s", applyStatusEnvVar, a.StatusPath)}
75+
if strings.TrimSpace(a.LogPath) != "" {
76+
env = append(env, fmt.Sprintf("%s=%s", applyLogEnvVar, a.LogPath))
77+
}
78+
return env
79+
}
80+
81+
func (a ApplyArtifacts) ClearStatus() error {
82+
if !a.Enabled() {
83+
return nil
84+
}
85+
if err := os.Remove(a.StatusPath); err != nil && !errors.Is(err, os.ErrNotExist) {
86+
return fmt.Errorf("remove upgrade helper status: %w", err)
87+
}
88+
return nil
89+
}
90+
91+
func (a ApplyArtifacts) RecordFailure(err error) error {
92+
if err == nil || !a.Enabled() {
93+
return nil
94+
}
95+
if mkdirErr := os.MkdirAll(filepath.Dir(a.StatusPath), 0o755); mkdirErr != nil {
96+
return fmt.Errorf("create upgrade helper state dir: %w", mkdirErr)
97+
}
98+
record := applyFailureRecord{
99+
Message: err.Error(),
100+
LogPath: strings.TrimSpace(a.LogPath),
101+
UpdatedAt: time.Now().UTC(),
102+
}
103+
data, marshalErr := json.MarshalIndent(record, "", " ")
104+
if marshalErr != nil {
105+
return fmt.Errorf("encode upgrade helper status: %w", marshalErr)
106+
}
107+
if writeErr := os.WriteFile(a.StatusPath, append(data, '\n'), 0o600); writeErr != nil {
108+
return fmt.Errorf("write upgrade helper status: %w", writeErr)
109+
}
110+
return nil
111+
}
112+
113+
func ConsumeApplyFailure(configPath string) (string, error) {
114+
artifacts, err := ResolveApplyArtifacts(configPath)
115+
if err != nil {
116+
return "", err
117+
}
118+
data, err := os.ReadFile(artifacts.StatusPath)
119+
if err != nil {
120+
if errors.Is(err, os.ErrNotExist) {
121+
return "", nil
122+
}
123+
return "", fmt.Errorf("read upgrade helper status: %w", err)
124+
}
125+
126+
var record applyFailureRecord
127+
if err := json.Unmarshal(data, &record); err != nil {
128+
return "", fmt.Errorf("decode upgrade helper status: %w", err)
129+
}
130+
if err := os.Remove(artifacts.StatusPath); err != nil && !errors.Is(err, os.ErrNotExist) {
131+
return "", fmt.Errorf("remove consumed upgrade helper status: %w", err)
132+
}
133+
134+
message := strings.TrimSpace(record.Message)
135+
if message == "" {
136+
return "", nil
137+
}
138+
if logPath := strings.TrimSpace(record.LogPath); logPath != "" {
139+
message = fmt.Sprintf("%s\nLog: %s", message, logPath)
140+
}
141+
return message, nil
142+
}
143+
144+
func applyArtifactsDir(configPath string) (string, error) {
145+
if path := strings.TrimSpace(configPath); path != "" {
146+
return filepath.Dir(path), nil
147+
}
148+
return config.DefaultDir()
149+
}

0 commit comments

Comments
 (0)