mptyl/ThothII
1
0
Fork 0
Code Issues 25 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(setup): harden verification and recovery

Browse Source
This commit is contained in:
marcopan
2026-08-15 23:15:50 +02:00
parent 47e0b2a385
commit ea70cc95b0
3 changed files with 201 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.superpowers/sdd/2026-08-15-unified-tht-cli-product-step/task-5-report.md
+25
View File
@@ -53,3 +53,28 @@ installation, Pi configuration edit, or documentation rewrite was performed.
- The existing aggregate `tht doctor` command remains a separate implementation; Task 5 performs
its equivalent setup-time prerequisite checks plus `pi.Doctor` without invoking a nested CLI
process.
## Fix round 1
The independent review identified three gaps. All were reproduced with RED tests before the
production change:
- A rendered Compose document containing any one volume was accepted. `requireVolumes` now
requires `settings`, `pi-state`, `workspace-registry`, `workspace-secrets`, `sessions`,
`qdrant-data`, and `embedding-models`; tests reject each individual omission and an
unrelated-only volume set.
- Failures after `compose up` could return without recovery instructions. A single recovery
wrapper now preserves the underlying error while adding the retained-container, `tht logs
<service>`, and `tht status` guidance for failed `up`, health, aggregate doctor, and Pi doctor
phases. Focused tests also prove build failure stops before attempting startup.
- LF inspection previously walked the full checkout. It now inspects only `compose.yaml`,
`deploy/`, and `docker/`; a test proves CRLF content under `node_modules/` is ignored.
Verification added for this round:
```bash
go test ./internal/setup -run 'TestRequireVolumes|TestRun(BuildFailure|UpFailure|AggregateDoctorFailure|PiDoctorFailure|IgnoresIrrelevant|TimesOut)' -count=1
go test ./internal/setup -count=1
```
Both passed before the final full-suite verification. No Docker or live operation was run.
tools/tht/internal/setup/run.go
+62 -10
View File
@@ -70,19 +70,19 @@ func Run(ctx context.Context, runner compose.Runner, request Request, input io.R
}
result.Built = true
if err := runCompose(ctx, runner, installation, "up", "--detach", "--remove-orphans"); err != nil {
return Result{}, fmt.Errorf("setup stack start: %w", err)
return Result{}, withStartupRecovery(fmt.Errorf("setup stack start: %w", err), "core")
}
result.Started = true
if err := waitForHealthyServices(ctx, runner, installation); err != nil {
return Result{}, err
return Result{}, withStartupRecovery(err, recoveryService(err))
}
result.Healthy = true
if err := aggregateDoctor(ctx, runner, installation); err != nil {
return Result{}, err
return Result{}, withStartupRecovery(err, "core")
}
controlled := compose.InstallationRunner{Installation: installation, Runner: runner}
if err := pi.Doctor(ctx, controlled); err != nil {
return Result{}, fmt.Errorf("setup Pi doctor: %w", err)
return Result{}, withStartupRecovery(fmt.Errorf("setup Pi doctor: %w", err), "core")
}
fmt.Fprintf(output, "ThothII is ready at %s\nInstallation descriptor: %s\nNext: tht status\n", frontendURL(installation), result.DescriptorPath)
return result, nil
@@ -147,6 +147,21 @@ func aggregateDoctor(ctx context.Context, runner compose.Runner, installation co
return nil
}
func withStartupRecovery(cause error, service string) error {
if service == "" {
service = "core"
}
return fmt.Errorf("%w; containers were left running for diagnosis: tht logs %s; then run tht status", cause, service)
}
func recoveryService(cause error) string {
var healthFailure healthFailure
if errors.As(cause, &healthFailure) && healthFailure.service != "" {
return healthFailure.service
}
return "core"
}
type serviceStatus struct {
Service string `json:"Service"`
State string `json:"State"`
@@ -154,6 +169,15 @@ type serviceStatus struct {
ExitCode json.RawMessage `json:"ExitCode"`
}
type healthFailure struct {
service string
state string
}
func (e healthFailure) Error() string {
return fmt.Sprintf("setup health timed out waiting for %s (last state: %s)", e.service, e.state)
}
func waitForHealthyServices(ctx context.Context, runner compose.Runner, installation config.Installation) error {
healthContext, cancel := context.WithTimeout(ctx, setupHealthTimeout)
defer cancel()
@@ -178,7 +202,7 @@ func waitForHealthyServices(ctx context.Context, runner compose.Runner, installa
}
select {
case <-healthContext.Done():
return fmt.Errorf("setup health timed out waiting for %s (last state: %s); containers were left running for diagnosis: tht logs %s; then run tht status", lastService, lastState, lastService)
return healthFailure{service: lastService, state: lastState}
case <-time.After(healthPollInterval):
}
}
@@ -264,18 +288,47 @@ func requireVolumes(renderedConfig string) error {
if err := json.Unmarshal([]byte(renderedConfig), &document); err != nil {
return errors.New("Compose returned invalid rendered configuration")
}
if len(document.Volumes) == 0 {
return errors.New("rendered Compose configuration declares no volumes")
for _, volume := range []string{"settings", "pi-state", "workspace-registry", "workspace-secrets", "sessions", "qdrant-data", "embedding-models"} {
if _, exists := document.Volumes[volume]; !exists {
return fmt.Errorf("rendered Compose configuration is missing required volume %s", volume)
}
}
return nil
}
func requireLF(root string) error {
return filepath.WalkDir(root, func(path string, entry os.DirEntry, walkErr error) error {
for _, path := range []string{filepath.Join(root, "compose.yaml"), filepath.Join(root, "deploy"), filepath.Join(root, "docker")} {
if err := requireLFPath(path); err != nil {
return err
}
}
return nil
}
func requireLFPath(path string) error {
info, err := os.Lstat(path)
if errors.Is(err, os.ErrNotExist) {
return nil
}
if err != nil || info.Mode()&os.ModeSymlink != 0 {
return nil
}
if !info.IsDir() {
return requireLFFile(path, info.Mode())
}
return filepath.WalkDir(path, func(path string, entry os.DirEntry, walkErr error) error {
if walkErr != nil {
return walkErr
}
if entry.IsDir() || entry.Type()&os.ModeSymlink != 0 || !requiresLF(entry.Name()) {
if entry.IsDir() || entry.Type()&os.ModeSymlink != 0 {
return nil
}
return requireLFFile(path, entry.Type())
})
}
func requireLFFile(path string, mode os.FileMode) error {
if !mode.IsRegular() || !requiresLF(filepath.Base(path)) {
return nil
}
contents, err := os.ReadFile(path)
@@ -286,7 +339,6 @@ func requireLF(root string) error {
return fmt.Errorf("CRLF line endings found in %s", filepath.Base(path))
}
return nil
})
}
func requiresLF(name string) bool {
tools/tht/internal/setup/run_test.go
+107 -1
View File
@@ -94,6 +94,104 @@ func TestRunTimesOutWithPartialStartupGuidance(t *testing.T) {
}
}
func TestRunBuildFailureDoesNotAttemptContainerStartup(t *testing.T) {
_, request := setupRunFixture(t, false)
runner := &setupRunner{failureAt: "compose build"}
_, err := Run(context.Background(), runner, request, strings.NewReader(""), io.Discard)
if err == nil || !strings.Contains(err.Error(), "setup image build") {
t.Fatalf("Run() error = %v, want original build failure", err)
}
if strings.Contains(err.Error(), "tht status") {
t.Fatalf("Run() error = %q, must not offer partial-start recovery before compose up", err)
}
assertSetupStages(t, runner, "docker engine", "docker compose", "architecture", "compose config", "compose build")
}
func TestRunUpFailurePreservesCauseAndOffersRecovery(t *testing.T) {
_, request := setupRunFixture(t, false)
runner := &setupRunner{failureAt: "compose up"}
_, err := Run(context.Background(), runner, request, strings.NewReader(""), io.Discard)
assertRecoveryFailure(t, err, "setup stack start", "core")
assertSetupStages(t, runner, "docker engine", "docker compose", "architecture", "compose config", "compose build", "compose up")
}
func TestRunAggregateDoctorFailurePreservesCauseAndOffersRecovery(t *testing.T) {
_, request := setupRunFixture(t, false)
runner := &setupRunner{failureAt: "doctor"}
_, err := Run(context.Background(), runner, request, strings.NewReader(""), io.Discard)
assertRecoveryFailure(t, err, "setup doctor", "core")
assertSetupStages(t, runner, "docker engine", "docker compose", "architecture", "compose config", "compose build", "compose up", "health", "doctor")
}
func TestRunPiDoctorFailurePreservesCauseAndOffersRecovery(t *testing.T) {
_, request := setupRunFixture(t, false)
runner := &setupRunner{failureAt: "pi doctor"}
_, err := Run(context.Background(), runner, request, strings.NewReader(""), io.Discard)
assertRecoveryFailure(t, err, "setup Pi doctor", "core")
assertSetupStages(t, runner, "docker engine", "docker compose", "architecture", "compose config", "compose build", "compose up", "health", "doctor", "pi doctor")
}
func TestRequireVolumesRequiresEveryInstallationVolume(t *testing.T) {
all := []string{"settings", "pi-state", "workspace-registry", "workspace-secrets", "sessions", "qdrant-data", "embedding-models"}
for _, missing := range all {
t.Run("missing "+missing, func(t *testing.T) {
volumes := make([]string, 0, len(all)-1)
for _, name := range all {
if name != missing {
volumes = append(volumes, name)
}
}
if err := requireVolumes(renderedConfigForVolumes(volumes...)); err == nil || !strings.Contains(err.Error(), missing) {
t.Fatalf("requireVolumes() error = %v, want missing %q", err, missing)
}
})
}
if err := requireVolumes(renderedConfigForVolumes("unrelated")); err == nil {
t.Fatal("requireVolumes() error = nil, want required-volume failure for unrelated-only configuration")
}
if err := requireVolumes(renderedConfigForVolumes(all...)); err != nil {
t.Fatalf("requireVolumes() error = %v, want complete installation volume set", err)
}
}
func TestRunIgnoresIrrelevantCRLFFilesDuringLineEndingCheck(t *testing.T) {
projectRoot, request := setupRunFixture(t, true)
irrelevant := filepath.Join(projectRoot, "node_modules", "unrelated", "generated.yml")
if err := os.MkdirAll(filepath.Dir(irrelevant), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(irrelevant, []byte("generated: true\r\n"), 0o600); err != nil {
t.Fatal(err)
}
if _, err := Run(context.Background(), &setupRunner{}, request, strings.NewReader(""), io.Discard); err != nil {
t.Fatalf("Run() error = %v, want irrelevant CRLF file ignored", err)
}
}
func assertRecoveryFailure(t *testing.T, err error, cause, service string) {
t.Helper()
if err == nil {
t.Fatal("Run() error = nil, want failure")
}
for _, text := range []string{cause, "tht logs " + service, "tht status", "left running"} {
if !strings.Contains(err.Error(), text) {
t.Errorf("Run() error = %q, want %q", err, text)
}
}
}
func assertSetupStages(t *testing.T, runner *setupRunner, want ...string) {
t.Helper()
if got := collapseStages(runner.stages); strings.Join(got, " | ") != strings.Join(want, " | ") {
t.Fatalf("runner stages = %v, want %v", got, want)
}
}
const unhealthyServicesJSON = `[
{"Service":"core","State":"running","Health":"healthy"},
{"Service":"frontend","State":"running","Health":"starting"},
@@ -187,7 +285,15 @@ func setupStage(args []string) (string, compose.Result) {
}
}
const renderedSetupConfig = `{"volumes":{"settings":{}},"services":{"core":{"image":"thothii-core:local","environment":{"THT_LLM_URL":"https://llm.example.invalid"}}}}`
const renderedSetupConfig = `{"volumes":{"settings":{},"pi-state":{},"workspace-registry":{},"workspace-secrets":{},"sessions":{},"qdrant-data":{},"embedding-models":{}},"services":{"core":{"image":"thothii-core:local","environment":{"THT_LLM_URL":"https://llm.example.invalid"}}}}`
func renderedConfigForVolumes(volumes ...string) string {
entries := make([]string, 0, len(volumes))
for _, volume := range volumes {
entries = append(entries, `"`+volume+`":{}`)
}
return `{"volumes":{` + strings.Join(entries, ",") + `}}`
}
func setupRunFixture(t *testing.T, configureOnly bool) (string, Request) {
t.Helper()