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

fix(cli): strengthen aggregate doctor diagnostics

Browse Source
This commit is contained in:
marcopan
2026-08-15 23:47:00 +02:00
parent 7ac99b4f7e
commit 0cb6a3f915
4 changed files with 232 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tools/tht/internal/doctor/report.go
+94 -15
View File
@@ -10,6 +10,7 @@ import (
"os"
"path/filepath"
"strings"
"time"
"github.com/aritmolab/thothii/tools/tht/internal/compose"
"github.com/aritmolab/thothii/tools/tht/internal/config"
@@ -24,6 +25,10 @@ const (
StatusSkipped = "skipped"
)
const probeTimeout = 5 * time.Second
const registryValidationProgram = `const fs=require("node:fs");const path="/data/workspace-registry/state/active.json";const s=JSON.parse(fs.readFileSync(path,"utf8"));const hex=/^[0-9a-f]{40}$/;if(!hex.test(s.head)||!Array.isArray(s.revisions)||s.revisions.some((r)=>!r||typeof r.id!=="string"||!r.id||!hex.test(r.commit)||!hex.test(r.blob))){process.exit(1)}for(const r of s.revisions){fs.accessSync("/data/workspace-registry/snapshots/"+r.commit+"/"+r.id+".yaml",fs.constants.R_OK)}`
// Check is one named, redacted diagnostic outcome.
type Check struct {
Name string `json:"name"`
@@ -42,14 +47,58 @@ type Runner interface {
Run(context.Context, []string, io.Reader) (compose.Result, error)
}
// HTTPProbeTarget identifies one local service endpoint that must answer a bounded request.
type HTTPProbeTarget struct {
Name string
Service string
URL string
}
// HTTPProbe is injectable so reachability failures remain independently testable.
type HTTPProbe interface {
Probe(context.Context, HTTPProbeTarget) error
}
type composeHTTPProbe struct {
installation config.Installation
runner Runner
}
func (p composeHTTPProbe) Probe(ctx context.Context, target HTTPProbeTarget) error {
probeContext, cancel := context.WithTimeout(ctx, probeTimeout)
defer cancel()
var command []string
switch target.Name {
case "core":
command = []string{"exec", "-T", target.Service, "curl", "-fsS", "--max-time", "5", target.URL}
case "frontend":
command = []string{"exec", "-T", target.Service, "wget", "-q", "-T", "5", "-O", "/dev/null", target.URL}
default:
return errors.New("unknown HTTP probe target")
}
result, err := p.runner.Run(probeContext, p.installation.ComposeArgs(command...), nil)
if err != nil {
return errors.New(commandDetail(target.Name+" HTTP probe", result, err, nil))
}
return nil
}
// Run performs diagnostics only. Expected environmental failures become failed checks so that
// callers can always render a complete report; unexpected local read errors are also reported.
func Run(ctx context.Context, installation config.Installation, runner Runner) (Report, error) {
return RunWithProbe(ctx, installation, runner, composeHTTPProbe{installation: installation, runner: runner})
}
// RunWithProbe performs diagnostics only, with an injectable bounded HTTP probe.
func RunWithProbe(ctx context.Context, installation config.Installation, runner Runner, probe HTTPProbe) (Report, error) {
if runner == nil {
return Report{}, errors.New("doctor requires a Docker command runner")
}
if probe == nil {
return Report{}, errors.New("doctor requires an HTTP probe")
}
secretValues := secretValues(installation)
report := Report{Checks: make([]Check, 0, 10)}
report := Report{Checks: make([]Check, 0, 11)}
add := func(name, status, detail string) {
report.Checks = append(report.Checks, Check{Name: name, Status: status, Detail: output.SanitizeDetail(detail, secretValues)})
}
@@ -59,12 +108,18 @@ func Run(ctx context.Context, installation config.Installation, runner Runner) (
} else {
add("descriptor", StatusPassed, "installation descriptor is loaded")
}
if err := filePermissions(installation); err != nil {
add("files", StatusFailed, err.Error())
} else {
add("files", StatusPassed, "declared host files have safe permissions")
}
if !commandCheck(ctx, runner, []string{"version", "--format", "{{.Client.Version}}"}, secretValues, add, "docker", "Docker Engine") {
add("compose", StatusSkipped, "Docker Engine is unavailable")
add("configuration", StatusSkipped, "Docker Engine is unavailable")
add("files", StatusSkipped, "Docker Engine is unavailable")
add("services", StatusSkipped, "Docker Engine is unavailable")
add("core-http", StatusSkipped, "core is unavailable")
add("frontend-http", StatusSkipped, "frontend is unavailable")
add("workspace-registry", StatusSkipped, "core is unavailable")
add("workflow", StatusSkipped, "core is unavailable")
add("pi", StatusSkipped, "core is unavailable")
@@ -72,8 +127,9 @@ func Run(ctx context.Context, installation config.Installation, runner Runner) (
}
if !commandCheck(ctx, runner, []string{"compose", "version", "--short"}, secretValues, add, "compose", "Docker Compose") {
add("configuration", StatusSkipped, "Docker Compose is unavailable")
add("files", StatusSkipped, "Docker Compose is unavailable")
add("services", StatusSkipped, "Docker Compose is unavailable")
add("core-http", StatusSkipped, "core is unavailable")
add("frontend-http", StatusSkipped, "frontend is unavailable")
add("workspace-registry", StatusSkipped, "core is unavailable")
add("workflow", StatusSkipped, "core is unavailable")
add("pi", StatusSkipped, "core is unavailable")
@@ -94,12 +150,6 @@ func Run(ctx context.Context, installation config.Installation, runner Runner) (
add("configuration", StatusPassed, "Compose configuration and required volumes are valid")
}
if err := filePermissions(installation); err != nil {
add("files", StatusFailed, err.Error())
} else {
add("files", StatusPassed, "declared host files have safe permissions")
}
status, statusAvailable := serviceStatus(ctx, installation, runner, secretValues, add)
coreRunning := false
if statusAvailable {
@@ -110,22 +160,51 @@ func Run(ctx context.Context, installation config.Installation, runner Runner) (
}
}
if !coreRunning {
add("core-http", StatusSkipped, "core is not running")
add("frontend-http", StatusSkipped, "core is not running")
add("workspace-registry", StatusSkipped, "core is not running")
add("workflow", StatusSkipped, "core is not running")
add("pi", StatusSkipped, "core is not running")
return finalize(report), nil
}
if configReady && workspaceRegistryConfigured(rendered) {
add("workspace-registry", StatusPassed, "workspace-registry volume is configured")
} else {
add("workspace-registry", StatusFailed, "workspace-registry volume is not configured")
}
reachabilityChecks(ctx, probe, secretValues, add)
registryCheck(ctx, installation, runner, secretValues, add, configReady, rendered)
workflowCheck(ctx, installation, runner, secretValues, add)
piCheck(ctx, installation, runner, secretValues, add)
return finalize(report), nil
}
func reachabilityChecks(ctx context.Context, probe HTTPProbe, secrets []string, add func(string, string, string)) {
for _, target := range []HTTPProbeTarget{
{Name: "core", Service: "core", URL: "http://127.0.0.1:8787/health"},
{Name: "frontend", Service: "frontend", URL: "http://127.0.0.1:8080/"},
} {
if err := probe.Probe(ctx, target); err != nil {
add(target.Name+"-http", StatusFailed, output.SanitizeDetail(err.Error(), secrets))
continue
}
add(target.Name+"-http", StatusPassed, target.Name+" answered a bounded HTTP probe")
}
}
func registryCheck(ctx context.Context, installation config.Installation, runner Runner, secrets []string, add func(string, string, string), configReady bool, rendered string) {
if !configReady {
add("workspace-registry", StatusFailed, "workspace-registry cannot be checked because Compose configuration is invalid")
return
}
if !workspaceRegistryDeclared(rendered) {
add("workspace-registry", StatusFailed, "workspace-registry volume is not configured")
return
}
result, err := runner.Run(ctx, installation.ComposeArgs("exec", "-T", "core", "node", "-e", registryValidationProgram), nil)
if err != nil {
add("workspace-registry", StatusFailed, commandDetail("container-local workspace registry", result, err, secrets))
return
}
add("workspace-registry", StatusPassed, "container-local active registry state and snapshots are valid")
}
func finalize(report Report) Report {
report.OK = len(report.Checks) > 0
for _, check := range report.Checks {
@@ -238,7 +317,7 @@ func ValidateVolumes(rendered string) error {
return nil
}
func workspaceRegistryConfigured(rendered string) bool {
func workspaceRegistryDeclared(rendered string) bool {
var document struct {
Volumes map[string]json.RawMessage `json:"volumes"`
}
tools/tht/internal/doctor/report_test.go
+74
View File
@@ -27,6 +27,22 @@ func TestRunReportsUnavailableDockerWithoutReturningAnExecutionError(t *testing.
}
}
// Catches Docker availability short-circuiting a host file-permission failure.
func TestRunChecksUnsafeFilesEvenWhenDockerIsUnavailable(t *testing.T) {
installation := doctorInstallation(t, "")
if err := os.Chmod(installation.EnvFile, 0o644); err != nil {
t.Fatal(err)
}
report, err := Run(context.Background(), installation, &doctorRunner{dockerUnavailable: true})
if err != nil {
t.Fatal(err)
}
if checkStatus(report, "files") != StatusFailed {
t.Fatalf("Run() files check = %q, want failed; report = %#v", checkStatus(report, "files"), report)
}
}
// Catches attempts to run in-container diagnostics when core is not running.
func TestRunSkipsContainerDiagnosticsWhenCoreIsStopped(t *testing.T) {
installation := doctorInstallation(t, "")
@@ -56,6 +72,7 @@ func TestRunUsesOnlyContainerLocalWorkflowAndPiDiagnosticsWhenCoreRuns(t *testin
if !report.OK || checkStatus(report, "workflow") != "passed" || checkStatus(report, "pi") != "passed" {
t.Fatalf("Run() report = %#v, want successful container diagnostics", report)
}
assertChecklist(t, report, []string{"descriptor", "files", "docker", "compose", "configuration", "services", "core-http", "frontend-http", "workspace-registry", "workflow", "pi"})
calls := strings.Join(runner.calls, "\n")
if !strings.Contains(calls, "exec -T core tht doctor --json") {
t.Fatalf("Run() calls = %s, want core-local workflow doctor", calls)
@@ -65,6 +82,40 @@ func TestRunUsesOnlyContainerLocalWorkflowAndPiDiagnosticsWhenCoreRuns(t *testin
}
}
// Catches a claimed valid registry based only on the rendered volume declaration.
func TestRunFailsAnInvalidContainerLocalRegistryState(t *testing.T) {
installation := doctorInstallation(t, "")
runner := &doctorRunner{services: healthyServices, registryInvalid: true}
report, err := Run(context.Background(), installation, runner)
if err != nil {
t.Fatal(err)
}
if report.OK || checkStatus(report, "workspace-registry") != StatusFailed {
t.Fatalf("Run() report = %#v, want failed registry diagnostic", report)
}
if !strings.Contains(strings.Join(runner.calls, "\n"), "workspace-registry/state/active.json") {
t.Fatalf("Run() calls = %v, want an actual registry-state read", runner.calls)
}
}
// Catches Compose health being treated as proof that the HTTP listeners answer requests.
func TestRunReportsEachHTTPReachabilityProbeFailure(t *testing.T) {
installation := doctorInstallation(t, "")
for _, endpoint := range []string{"core", "frontend"} {
t.Run(endpoint, func(t *testing.T) {
probe := &probeStub{failures: map[string]error{endpoint: errors.New(endpoint + " unreachable")}}
report, err := RunWithProbe(context.Background(), installation, &doctorRunner{services: healthyServices}, probe)
if err != nil {
t.Fatal(err)
}
if report.OK || checkStatus(report, endpoint+"-http") != StatusFailed {
t.Fatalf("RunWithProbe() report = %#v, want failed %s HTTP check", report, endpoint)
}
})
}
}
// Catches a workflow failure leaking a credential from a declared secret file into a report.
func TestRunRedactsWorkflowDiagnosticFailures(t *testing.T) {
installation := doctorInstallation(t, "WORKFLOW_TOKEN_FILE=%s\n")
@@ -124,6 +175,7 @@ type doctorRunner struct {
dockerUnavailable bool
services string
workflowFailure string
registryInvalid bool
}
func (r *doctorRunner) Run(_ context.Context, args []string, _ io.Reader) (compose.Result, error) {
@@ -151,6 +203,11 @@ func (r *doctorRunner) Run(_ context.Context, args []string, _ io.Reader) (compo
return compose.Result{Stderr: r.workflowFailure, ExitCode: 23}, errors.New("workflow failed")
}
return compose.Result{Stdout: `{"ok":true,"checks":[]}`}, nil
case strings.Contains(call, "workspace-registry/state/active.json"):
if r.registryInvalid {
return compose.Result{ExitCode: 23, Stderr: "invalid workspace registry"}, errors.New("registry invalid")
}
return compose.Result{Stdout: "registry is valid\n"}, nil
case strings.Contains(call, "pi --version") || strings.Contains(call, "PI_VERSION") || strings.Contains(call, "io.thothii.pi.version"):
return compose.Result{Stdout: "0.80.3\n"}, nil
case strings.Contains(call, "test -w /home/thoth/.pi") || strings.Contains(call, "test -r /home/thoth/.pi/agent/auth.json") || strings.Contains(call, "/health"):
@@ -163,6 +220,12 @@ func (r *doctorRunner) Run(_ context.Context, args []string, _ io.Reader) (compo
return compose.Result{}, nil
}
type probeStub struct{ failures map[string]error }
func (p *probeStub) Probe(_ context.Context, endpoint HTTPProbeTarget) error {
return p.failures[endpoint.Name]
}
func checkStatus(report Report, name string) string {
for _, check := range report.Checks {
if check.Name == name {
@@ -180,6 +243,17 @@ func reportText(report Report) string {
return strings.Join(parts, "\n")
}
func assertChecklist(t *testing.T, report Report, want []string) {
t.Helper()
got := make([]string, 0, len(report.Checks))
for _, check := range report.Checks {
got = append(got, check.Name)
}
if strings.Join(got, ",") != strings.Join(want, ",") {
t.Fatalf("doctor checklist = %v, want %v", got, want)
}
}
const renderedConfig = `{"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"}}}}`
const healthyServices = `[