From 28db30bd78f65ad7aaa785f09795c2d806cd0bd5 Mon Sep 17 00:00:00 2001 From: User Date: Mon, 7 Sep 2026 01:15:28 +0200 Subject: [PATCH] fix(ops): make server diagnostics release-safe --- backend/src/operator-command.ts | 59 ++++++++----- backend/test/operator-command.test.ts | 82 +++++++++++++++++ docker/tht.Dockerfile | 20 +++-- scripts/build-tht.sh | 13 ++- scripts/install-tht.sh | 0 scripts/test-install-tht.sh | 2 + scripts/test-tht-build-contract.sh | 16 +++- .../backup/restore_projection_linux_test.go | 1 + tools/tht/internal/doctor/report.go | 88 +++++++++++++++++-- tools/tht/internal/doctor/report_test.go | 54 ++++++++++++ 10 files changed, 297 insertions(+), 38 deletions(-) create mode 100644 backend/test/operator-command.test.ts mode change 100644 => 100755 scripts/install-tht.sh diff --git a/backend/src/operator-command.ts b/backend/src/operator-command.ts index 2fd3bf74..d1408311 100644 --- a/backend/src/operator-command.ts +++ b/backend/src/operator-command.ts @@ -16,6 +16,8 @@ import { loadSettings } from "./settings/settings-store.js"; import { ThtRunner, type SessionRow } from "./tht/tht-runner.js"; import { WorkspaceRegistry } from "./workspaces/registry.js"; import { WorkspaceSecretStore } from "./workspaces/secret-store.js"; +import { createCatalogRepository } from "./catalog/repository.js"; +import type { CatalogRepository } from "./catalog/types.js"; type OperatorAction = "maintenance-activate" | "maintenance-deactivate" | "maintenance-status" | "session-inventory" | "workflow-doctor" | "workspace-integrity" @@ -30,7 +32,7 @@ const lifecyclePrincipal: PrincipalContext = { isAdmin: true, }; -function operatorRunner(config: AppConfig): ThtRunner { +function operatorRunner(config: AppConfig, catalogRepository: CatalogRepository): ThtRunner { const workspaceSecretStore = new WorkspaceSecretStore({ root: config.workspaceSecretStoreRoot, runtimeRoot: config.workspaceSecretRuntimeRoot, @@ -46,6 +48,7 @@ function operatorRunner(config: AppConfig): ThtRunner { secretsFile: config.secretsFile, secretFiles: config.secretFiles, workspaceSecretStore, + catalogRepository, semanticRuntime: { internalQdrantUrl: config.internalQdrantUrl, internalEmbeddingUrl: config.internalEmbeddingUrl, @@ -55,36 +58,50 @@ function operatorRunner(config: AppConfig): ThtRunner { }).withPrincipal(lifecyclePrincipal); } +async function withOperatorRunner( + config: AppConfig, + operation: (runner: ThtRunner) => Promise, +): Promise { + const catalogRepository = createCatalogRepository(config.catalogDatabase); + try { + return await operation(operatorRunner(config, catalogRepository)); + } finally { + await catalogRepository.close?.(); + } +} + async function sessionInventory(config: AppConfig): Promise>> { const registry = new WorkspaceRegistry(config.workspaceRegistry); const revisions = await registry.listRetainedSnapshots(); - const runner = operatorRunner(config); - const sessions = new Map(); - for (const revision of revisions) { - for (const session of await runner.sessionList(revision.snapshotPath)) sessions.set(session.id, session); - } - return [...sessions.values()].map(({ status, archived }) => ({ status, archived: archived === true })); + return await withOperatorRunner(config, async (runner) => { + const sessions = new Map(); + for (const revision of revisions) { + for (const session of await runner.sessionList(revision.snapshotPath)) sessions.set(session.id, session); + } + return [...sessions.values()].map(({ status, archived }) => ({ status, archived: archived === true })); + }); } async function workflowDiagnostics(config: AppConfig): Promise<{ ready: true; workspaces: number }> { const registry = new WorkspaceRegistry(config.workspaceRegistry); const revisions = await registry.listRetainedSnapshots(); if (revisions.length === 0) throw new Error("workflow diagnostics unavailable"); - const runner = operatorRunner(config); - for (const revision of revisions) { - const result = await runner.run(["doctor", "--json"], revision.snapshotPath); - let payload: unknown; - try { - payload = JSON.parse(result.stdout); - } catch { - throw new Error("workflow diagnostics failed"); + return await withOperatorRunner(config, async (runner) => { + for (const revision of revisions) { + const result = await runner.run(["doctor", "--json"], revision.snapshotPath); + let payload: unknown; + try { + payload = JSON.parse(result.stdout); + } catch { + throw new Error("workflow diagnostics failed"); + } + if ( + result.code !== 0 || !payload || typeof payload !== "object" + || (payload as { ok?: unknown }).ok !== true + ) throw new Error("workflow diagnostics failed"); } - if ( - result.code !== 0 || !payload || typeof payload !== "object" - || (payload as { ok?: unknown }).ok !== true - ) throw new Error("workflow diagnostics failed"); - } - return { ready: true, workspaces: revisions.length }; + return { ready: true, workspaces: revisions.length }; + }); } async function workspaceIntegrity(config: AppConfig): Promise<{ diff --git a/backend/test/operator-command.test.ts b/backend/test/operator-command.test.ts new file mode 100644 index 00000000..5661d222 --- /dev/null +++ b/backend/test/operator-command.test.ts @@ -0,0 +1,82 @@ +import { beforeEach, expect, test, vi } from "vitest"; +import type { AppConfig } from "../src/config.js"; + +const fakes = vi.hoisted(() => ({ + catalogRepository: { close: vi.fn(async () => {}) }, + createCatalogRepository: vi.fn(), + runnerConfig: undefined as Record | undefined, + run: vi.fn(async () => ({ + code: 0, + stdout: JSON.stringify({ ok: true }), + stderr: "", + })), +})); + +vi.mock("../src/catalog/repository.js", () => ({ + createCatalogRepository: fakes.createCatalogRepository, +})); + +vi.mock("../src/tht/tht-runner.js", () => ({ + ThtRunner: class { + constructor(config: Record) { + fakes.runnerConfig = config; + } + + run = fakes.run; + + withPrincipal() { + return this; + } + }, +})); + +vi.mock("../src/workspaces/registry.js", () => ({ + WorkspaceRegistry: class { + async listRetainedSnapshots() { + return [{ snapshotPath: "/data/workspace-registry/snapshots/revision/workspace.yaml" }]; + } + }, +})); + +vi.mock("../src/workspaces/secret-store.js", () => ({ + WorkspaceSecretStore: class {}, +})); + +import { runOperatorAction } from "../src/operator-command.js"; + +const config = { + catalogDatabase: { host: "catalog-db" }, + workspaceSecretStoreRoot: "/data/workspace-secrets", + workspaceSecretRuntimeRoot: "/tmp/workspace-secrets", + workspaceRegistry: { + installationId: "test", + root: "/data/workspace-registry", + secretRoots: ["/run/secrets"], + }, + thtBin: "/opt/venv/bin/tht", + harnessDir: "/app/harness", + dataRoot: "/data", + internalQdrantUrl: "http://qdrant:6333", + internalEmbeddingUrl: "http://embedding:11434", + internalEmbeddingModel: "qwen3-embedding:0.6b", + internalEmbeddingDimensions: 1024, +} as AppConfig; + +beforeEach(() => { + fakes.catalogRepository.close.mockClear(); + fakes.createCatalogRepository.mockReset(); + fakes.createCatalogRepository.mockReturnValue(fakes.catalogRepository); + fakes.run.mockClear(); + fakes.runnerConfig = undefined; +}); + +test("workflow doctor gives schema-v4 runtime rendering a live Catalog repository", async () => { + await expect(runOperatorAction("workflow-doctor", config)).resolves.toEqual({ + ready: true, + workspaces: 1, + }); + + expect(fakes.createCatalogRepository).toHaveBeenCalledWith(config.catalogDatabase); + expect(fakes.runnerConfig?.catalogRepository).toBe(fakes.catalogRepository); + expect(fakes.catalogRepository.close).toHaveBeenCalledOnce(); +}); diff --git a/docker/tht.Dockerfile b/docker/tht.Dockerfile index 6d6544c9..ed7baac2 100644 --- a/docker/tht.Dockerfile +++ b/docker/tht.Dockerfile @@ -5,12 +5,20 @@ COPY tools/tht/go.mod tools/tht/go.sum ./ RUN go mod download COPY tools/tht ./ -RUN mkdir -p /out \ - && CGO_ENABLED=0 GOOS=windows GOARCH=amd64 go build -trimpath -ldflags='-s -w' -o /out/tht-windows-amd64.exe ./cmd/tht \ - && CGO_ENABLED=0 GOOS=darwin GOARCH=amd64 go build -trimpath -ldflags='-s -w' -o /out/tht-darwin-amd64 ./cmd/tht \ - && CGO_ENABLED=0 GOOS=darwin GOARCH=arm64 go build -trimpath -ldflags='-s -w' -o /out/tht-darwin-arm64 ./cmd/tht \ - && CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -trimpath -ldflags='-s -w' -o /out/tht-linux-amd64 ./cmd/tht \ - && CGO_ENABLED=0 GOOS=linux GOARCH=arm64 go build -trimpath -ldflags='-s -w' -o /out/tht-linux-arm64 ./cmd/tht +ARG THT_VERSION=0.0.0-dev +ARG THT_COMMIT=unknown +ARG THT_BUILD_TIME=unknown + +RUN linker_flags="-s -w \ + -X github.com/aritmolab/thothii/tools/tht/internal/version.semanticVersion=${THT_VERSION} \ + -X github.com/aritmolab/thothii/tools/tht/internal/version.commit=${THT_COMMIT} \ + -X github.com/aritmolab/thothii/tools/tht/internal/version.buildTime=${THT_BUILD_TIME}" \ + && mkdir -p /out \ + && CGO_ENABLED=0 GOOS=windows GOARCH=amd64 go build -trimpath -ldflags="$linker_flags" -o /out/tht-windows-amd64.exe ./cmd/tht \ + && CGO_ENABLED=0 GOOS=darwin GOARCH=amd64 go build -trimpath -ldflags="$linker_flags" -o /out/tht-darwin-amd64 ./cmd/tht \ + && CGO_ENABLED=0 GOOS=darwin GOARCH=arm64 go build -trimpath -ldflags="$linker_flags" -o /out/tht-darwin-arm64 ./cmd/tht \ + && CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -trimpath -ldflags="$linker_flags" -o /out/tht-linux-amd64 ./cmd/tht \ + && CGO_ENABLED=0 GOOS=linux GOARCH=arm64 go build -trimpath -ldflags="$linker_flags" -o /out/tht-linux-arm64 ./cmd/tht FROM scratch AS export COPY --from=build /out/ / diff --git a/scripts/build-tht.sh b/scripts/build-tht.sh index 27231ff8..48aeb954 100755 --- a/scripts/build-tht.sh +++ b/scripts/build-tht.sh @@ -4,6 +4,11 @@ export DOCKER_BUILDKIT=1 repository_root=$(cd "$(dirname "$0")/.." && pwd) output_directory="${THT_THT_OUTPUT_DIRECTORY:-$repository_root/dist/tht}" +build_commit=$(git -C "$repository_root" rev-parse HEAD) +build_time=$(git -C "$repository_root" show -s --format=%cI HEAD) +if ! build_version=$(git -C "$repository_root" describe --tags --exact-match HEAD 2>/dev/null); then + build_version=0.0.0-dev +fi usage() { cat <<'EOF' @@ -43,4 +48,10 @@ if [[ "$output_directory" != /* || "$output_directory" == / || "$output_director fi mkdir -p "$output_directory" -docker build --file "$repository_root/docker/tht.Dockerfile" --output "type=local,dest=$output_directory" "$repository_root" +docker build \ + --build-arg "THT_VERSION=$build_version" \ + --build-arg "THT_COMMIT=$build_commit" \ + --build-arg "THT_BUILD_TIME=$build_time" \ + --file "$repository_root/docker/tht.Dockerfile" \ + --output "type=local,dest=$output_directory" \ + "$repository_root" diff --git a/scripts/install-tht.sh b/scripts/install-tht.sh old mode 100644 new mode 100755 diff --git a/scripts/test-install-tht.sh b/scripts/test-install-tht.sh index a2c24d6f..403af28b 100644 --- a/scripts/test-install-tht.sh +++ b/scripts/test-install-tht.sh @@ -11,6 +11,8 @@ fail() { exit 1 } +test -x "$installer" || fail "install-tht.sh must be executable for the documented invocation" + sha256() { if command -v sha256sum >/dev/null 2>&1; then sha256sum "$1" | awk '{print $1}' diff --git a/scripts/test-tht-build-contract.sh b/scripts/test-tht-build-contract.sh index 7fbfd697..d01ac2d7 100755 --- a/scripts/test-tht-build-contract.sh +++ b/scripts/test-tht-build-contract.sh @@ -22,7 +22,16 @@ printf '%s\n' "$manifest" | grep -Eq 'Platform:[[:space:]]+linux/arm64' temporary_output=$(mktemp -d) trap 'rm -rf "$temporary_output"' EXIT HUP INT TERM -docker build --file "$dockerfile" --output "type=local,dest=$temporary_output" "$repository_root" >/dev/null +test_version='9.8.7-test' +test_commit='0123456789abcdef0123456789abcdef01234567' +test_build_time='2026-09-07T01:00:00+02:00' +docker build \ + --build-arg "THT_VERSION=$test_version" \ + --build-arg "THT_COMMIT=$test_commit" \ + --build-arg "THT_BUILD_TIME=$test_build_time" \ + --file "$dockerfile" \ + --output "type=local,dest=$temporary_output" \ + "$repository_root" >/dev/null test -s "$temporary_output/tht-windows-amd64.exe" test -s "$temporary_output/tht-darwin-amd64" @@ -30,4 +39,9 @@ test -s "$temporary_output/tht-darwin-arm64" test -s "$temporary_output/tht-linux-amd64" test -s "$temporary_output/tht-linux-arm64" +version_json=$($temporary_output/tht-linux-amd64 version --json) +printf '%s\n' "$version_json" | grep -Fq '"version":"9.8.7-test"' +printf '%s\n' "$version_json" | grep -Fq '"commit":"0123456789abcdef0123456789abcdef01234567"' +printf '%s\n' "$version_json" | grep -Fq '"buildTime":"2026-09-07T01:00:00+02:00"' + echo "tht build contract passed." diff --git a/tools/tht/internal/backup/restore_projection_linux_test.go b/tools/tht/internal/backup/restore_projection_linux_test.go index cc0a5a32..75feab00 100644 --- a/tools/tht/internal/backup/restore_projection_linux_test.go +++ b/tools/tht/internal/backup/restore_projection_linux_test.go @@ -14,6 +14,7 @@ import ( func TestRestoreAuthBearingArchiveRefusesNonRootBeforeTransactionOrWrite(t *testing.T) { installation, archive := projectedRestoreFixture(t) deps := restoreTestDependencies(t, newBackupRunner(installation, false)) + deps.requireAuthProjection = requireAuthProjectionRestorePrivilege checkpointCalled := false beginCalled := false writeCalled := false diff --git a/tools/tht/internal/doctor/report.go b/tools/tht/internal/doctor/report.go index 54cd5823..f2b68fd3 100644 --- a/tools/tht/internal/doctor/report.go +++ b/tools/tht/internal/doctor/report.go @@ -373,27 +373,97 @@ func filePermissions(installation config.Installation) error { return nil } -// ValidateVolumes checks the eight persistent volumes required by a local ThothII installation. +type renderedMount struct { + Type string `json:"type"` + Target string `json:"target"` + ReadOnly bool `json:"read_only"` +} + +type renderedService struct { + Volumes []renderedMount `json:"volumes"` +} + +type renderedPersistence struct { + Volumes map[string]json.RawMessage `json:"volumes"` + Services map[string]renderedService `json:"services"` +} + +var requiredNamedVolumes = []string{ + "settings", "pi-state", "workspace-registry", "workspace-secrets", + "sessions", "qdrant-data", "embedding-models", "auth-state", +} + +var requiredPersistentMounts = []struct { + service string + target string + label string +}{ + {service: "core", target: "/data/settings", label: "settings"}, + {service: "core", target: "/home/thoth/.pi", label: "pi-state"}, + {service: "core", target: "/data/workspace-registry", label: "workspace-registry"}, + {service: "core", target: "/data/workspace-secrets", label: "workspace-secrets"}, + {service: "core", target: "/data/sessions", label: "sessions"}, + {service: "qdrant", target: "/qdrant/storage", label: "qdrant-data"}, + {service: "embedding", target: "/root/.ollama", label: "embedding-models"}, + {service: "core", target: "/data/auth", label: "auth-state"}, +} + +// ValidateVolumes accepts either the portable named-volume layout or the server layout where a +// writable bind root owns several nested persistence paths. Docker Compose omits unused top-level +// volume declarations after a server override, so declarations alone cannot validate that profile. func ValidateVolumes(rendered string) error { - var document struct { - Volumes map[string]json.RawMessage `json:"volumes"` - } + var document renderedPersistence if err := json.Unmarshal([]byte(rendered), &document); err != nil { return errors.New("Compose returned invalid rendered configuration") } - for _, name := range []string{"settings", "pi-state", "workspace-registry", "workspace-secrets", "sessions", "qdrant-data", "embedding-models", "auth-state"} { + missing := "" + for _, name := range requiredNamedVolumes { if _, exists := document.Volumes[name]; !exists { - return fmt.Errorf("rendered Compose configuration is missing required volume %s", name) + missing = name + break + } + } + if missing == "" { + return nil + } + if len(document.Services) == 0 { + return fmt.Errorf("rendered Compose configuration is missing required volume %s", missing) + } + for _, required := range requiredPersistentMounts { + service, exists := document.Services[required.service] + if !exists || !hasWritableMountCovering(service.Volumes, required.target) { + return fmt.Errorf("rendered Compose configuration is missing persistent mount %s", required.label) } } return nil } func workspaceRegistryDeclared(rendered string) bool { - var document struct { - Volumes map[string]json.RawMessage `json:"volumes"` + var document renderedPersistence + if json.Unmarshal([]byte(rendered), &document) != nil { + return false } - return json.Unmarshal([]byte(rendered), &document) == nil && document.Volumes["workspace-registry"] != nil + if document.Volumes["workspace-registry"] != nil { + return true + } + return hasWritableMountCovering(document.Services["core"].Volumes, "/data/workspace-registry") +} + +func hasWritableMountCovering(mounts []renderedMount, target string) bool { + for _, mount := range mounts { + if mount.ReadOnly || (mount.Type != "bind" && mount.Type != "volume") { + continue + } + mountTarget := filepath.Clean(mount.Target) + if !filepath.IsAbs(mountTarget) { + continue + } + relative, err := filepath.Rel(mountTarget, target) + if err == nil && relative != ".." && !strings.HasPrefix(relative, ".."+string(filepath.Separator)) { + return true + } + } + return false } func commandDetail(label string, result compose.Result, err error, secrets []string) string { diff --git a/tools/tht/internal/doctor/report_test.go b/tools/tht/internal/doctor/report_test.go index e6618026..41f83241 100644 --- a/tools/tht/internal/doctor/report_test.go +++ b/tools/tht/internal/doctor/report_test.go @@ -89,6 +89,38 @@ func TestValidateVolumesRequiresAuthState(t *testing.T) { } } +// Catches rejecting a server installation whose durable state is provided by bind mounts. +func TestRunAcceptsServerBindMountPersistence(t *testing.T) { + installation := doctorInstallation(t, "") + installation.Profile = "server" + runner := &doctorRunner{services: healthyServices, rendered: serverRenderedConfig} + + report, err := Run(context.Background(), installation, runner) + if err != nil { + t.Fatal(err) + } + if !report.OK { + t.Fatalf("Run() report = %#v, want healthy server bind-mount installation", report) + } + if checkStatus(report, "configuration") != StatusPassed || + checkStatus(report, "authentication") != StatusPassed || + checkStatus(report, "workspace-registry") != StatusPassed { + t.Fatalf("Run() report = %#v, want server configuration and dependent checks passed", report) + } +} + +func TestValidateVolumesRejectsIncompleteServerBindMountPersistence(t *testing.T) { + withoutPiState := strings.Replace( + serverRenderedConfig, + `{"type": "bind", "source": "/srv/thothii/pi-state", "target": "/home/thoth/.pi"}`, + `{"type": "bind", "source": "/srv/thothii/pi-state", "target": "/tmp/pi-state"}`, + 1, + ) + if err := ValidateVolumes(withoutPiState); err == nil || !strings.Contains(err.Error(), "pi-state") { + t.Fatalf("ValidateVolumes() error = %v, want missing pi-state persistence", err) + } +} + // Catches Docker availability short-circuiting a host file-permission failure. func TestRunChecksUnsafeFilesEvenWhenDockerIsUnavailable(t *testing.T) { installation := doctorInstallation(t, "") @@ -302,6 +334,7 @@ type doctorRunner struct { calls []string dockerUnavailable bool services string + rendered string workflowFailure string registryInvalid bool } @@ -320,6 +353,9 @@ func (r *doctorRunner) Run(_ context.Context, args []string, _ io.Reader) (compo case strings.Contains(call, "config --quiet"): return compose.Result{}, nil case strings.Contains(call, "config --format json"): + if r.rendered != "" { + return compose.Result{Stdout: r.rendered}, nil + } return compose.Result{Stdout: renderedConfig}, nil case strings.Contains(call, "ps --all --format json"): if r.services == "" { @@ -395,6 +431,24 @@ func assertChecklist(t *testing.T, report Report, want []string) { const renderedConfig = `{"volumes":{"settings":{},"pi-state":{},"workspace-registry":{},"workspace-secrets":{},"sessions":{},"qdrant-data":{},"embedding-models":{},"auth-state":{}},"services":{"core":{"image":"thothii-core:local","environment":{"THT_LLM_URL":"https://llm.example.invalid"}}}}` +const serverRenderedConfig = `{ + "volumes": {"catalog-data": {}, "qdrant-data": {}, "embedding-models": {}}, + "services": { + "core": { + "image": "thothii-core:server", + "environment": {"THT_LLM_URL": "https://llm.example.invalid"}, + "volumes": [ + {"type": "bind", "source": "/srv/thothii/data", "target": "/data"}, + {"type": "bind", "source": "/srv/thothii/pi-state", "target": "/home/thoth/.pi"}, + {"type": "bind", "source": "/srv/thothii/workspace-registry", "target": "/data/workspace-registry"} + ] + }, + "catalog-db": {"volumes": [{"type": "volume", "source": "catalog-data", "target": "/var/lib/postgresql/data"}]}, + "qdrant": {"volumes": [{"type": "volume", "source": "qdrant-data", "target": "/qdrant/storage"}]}, + "embedding": {"volumes": [{"type": "volume", "source": "embedding-models", "target": "/root/.ollama"}]} + } +}` + const healthyServices = `[ {"Service":"core","State":"running","Health":"healthy"}, {"Service":"frontend","State":"running","Health":"healthy"},