Stable error codes, audit manifest migration tests, and full-binary release smoke tests

internal/audit/migration_test.go

@@ -0,0 +1,145 @@
+// Copyright 2026 Glassbox Users
+// SPDX-License-Identifier: Apache-2.0
+
+// migration_test.go covers the audit segment manifest's schema_version
+// compatibility contract [Issue #872]: manifests written before the
+// schema_version field existed must still verify (forward loading of
+// legacy artifacts), and manifests declaring a schema_version this binary
+// does not recognize must be rejected rather than silently accepted.
+
+package audit
+
+import (
+	"os"
+	"strings"
+	"testing"
+)
+
+// TestVerifyDirectory_LegacyManifestNoSchemaVersion_StillValid simulates a
+// segment manifest written by a pre-versioning build of Glassbox (no
+// schema_version field, so it deserializes as ""). VerifyDirectory must still
+// treat the segment as valid: SchemaVersion is empty-tolerant by design (see
+// verify.go's `manifestSV != "" && manifestSV != SchemaVersion` check), which
+// is the compatibility guarantee this test pins down.
+func TestVerifyDirectory_LegacyManifestNoSchemaVersion_StillValid(t *testing.T) {
+	dir := t.TempDir()
+	w, err := OpenWriter(dir, RotationConfig{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := w.WriteRecord([]byte(`{"n":1}`)); err != nil {
+		t.Fatal(err)
+	}
+	if err := w.Rotate(); err != nil {
+		t.Fatal(err)
+	}
+	if err := w.Close(); err != nil {
+		t.Fatal(err)
+	}
+
+	segments, err := ListSegments(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(segments) != 1 {
+		t.Fatalf("need 1 segment, got %d", len(segments))
+	}
+
+	// Rewrite the manifest with schema_version stripped, as a legacy
+	// (pre-versioning) manifest would look on disk.
+	m, err := ReadManifest(segments[0].ManifestPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	m.SchemaVersion = ""
+	if err := WriteManifestAtomic(segments[0].ManifestPath, *m); err != nil {
+		t.Fatal(err)
+	}
+
+	result, err := VerifyDirectory(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !result.Valid || !result.ChainValid {
+		t.Fatalf("expected legacy (schema_version-less) manifest to verify, got %+v", result)
+	}
+}
+
+// TestVerifyDirectory_UnsupportedSchemaVersion_Rejected simulates a manifest
+// declaring a schema_version this binary does not recognize (e.g. written by
+// a future Glassbox release). VerifyDirectory must flag it as invalid rather
+// than silently accepting an artifact it cannot fully interpret.
+func TestVerifyDirectory_UnsupportedSchemaVersion_Rejected(t *testing.T) {
+	dir := t.TempDir()
+	w, err := OpenWriter(dir, RotationConfig{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := w.WriteRecord([]byte(`{"n":1}`)); err != nil {
+		t.Fatal(err)
+	}
+	if err := w.Rotate(); err != nil {
+		t.Fatal(err)
+	}
+	if err := w.Close(); err != nil {
+		t.Fatal(err)
+	}
+
+	segments, err := ListSegments(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(segments) != 1 {
+		t.Fatalf("need 1 segment, got %d", len(segments))
+	}
+
+	m, err := ReadManifest(segments[0].ManifestPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	m.SchemaVersion = "99"
+	if err := WriteManifestAtomic(segments[0].ManifestPath, *m); err != nil {
+		t.Fatal(err)
+	}
+
+	result, err := VerifyDirectory(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if result.Valid {
+		t.Fatal("expected verification failure for unsupported schema_version")
+	}
+	found := false
+	for _, issue := range result.Issues {
+		if strings.Contains(issue, "unsupported schema_version") {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Fatalf("expected unsupported schema_version issue, got %v", result.Issues)
+	}
+}
+
+// TestReadManifest_Roundtrip_PreservesSchemaVersion is a narrow sanity check
+// that ReadManifest/WriteManifestAtomic roundtrip schema_version faithfully,
+// which the two tests above depend on to construct their fixtures.
+func TestReadManifest_Roundtrip_PreservesSchemaVersion(t *testing.T) {
+	dir := t.TempDir()
+	path := dir + string(os.PathSeparator) + "segment-000001-20260101T000000Z.manifest.json"
+	m := SegmentManifest{
+		SchemaVersion: SchemaVersion,
+		Segment:       "segment-000001-20260101T000000Z.jsonl",
+		Sequence:      1,
+	}
+	if err := WriteManifestAtomic(path, m); err != nil {
+		t.Fatal(err)
+	}
+	got, err := ReadManifest(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got.SchemaVersion != SchemaVersion {
+		t.Fatalf("SchemaVersion roundtrip = %q, want %q", got.SchemaVersion, SchemaVersion)
+	}
+}

internal/cmd/errcode_testhelpers_test.go

@@ -0,0 +1,59 @@
+// Copyright 2026 Glassbox Users
+// SPDX-License-Identifier: Apache-2.0
+
+// errcode_testhelpers_test.go provides a shared assertion for the stable
+// error-code contract [Issue #762]: every error a command returns must be
+// (or wrap) an *errors.ErstError, so that automation can rely on a stable
+// Code and a consistent exit-code bucket instead of receiving an
+// unclassified error that surfaces as ErstUnknown.
+
+package cmd
+
+import (
+	stderrors "errors"
+	"testing"
+
+	glassboxerrors "github.com/dotandev/glassbox/internal/errors"
+)
+
+// requireErstError fails the test unless err is non-nil and is (or wraps) an
+// *errors.ErstError. It returns the unwrapped *ErstError for further
+// assertions (e.g. on Code or Hint) when the caller needs them.
+//
+// Use this at the boundary of any command's RunE (or the helper it delegates
+// to) to guard against bare, unclassified errors leaking to the CLI's error
+// output, which would otherwise report as ErstUnknown in both JSON and text
+// modes.
+func requireErstError(t *testing.T, err error) *glassboxerrors.ErstError {
+	t.Helper()
+	if err == nil {
+		t.Fatal("requireErstError: expected a non-nil error")
+	}
+	var e *glassboxerrors.ErstError
+	if !stderrors.As(err, &e) {
+		t.Fatalf("requireErstError: error is not an *ErstError (unclassified command error): %T — %v", err, err)
+	}
+	if e.Code == "" {
+		t.Fatalf("requireErstError: *ErstError has an empty Code: %v", err)
+	}
+	return e
+}
+
+// requireErstErrorCode is requireErstError plus an assertion that the code
+// matches want.
+func requireErstErrorCode(t *testing.T, err error, want glassboxerrors.ErstErrorCode) *glassboxerrors.ErstError {
+	t.Helper()
+	e := requireErstError(t, err)
+	if e.Code != want {
+		t.Fatalf("requireErstErrorCode: Code = %q, want %q", e.Code, want)
+	}
+	return e
+}
+
+// TestWrapInternalIsErstError guards the note-add ID-generation failure path
+// (previously a bare fmt.Errorf) against regressing to an unclassified error.
+func TestWrapInternalIsErstError(t *testing.T) {
+	wrapped := glassboxerrors.WrapInternal("failed to generate note ID", stderrors.New("boom"))
+	requireErstErrorCode(t, wrapped, glassboxerrors.ErstInternalError)
+}
+

internal/cmd/note.go

@@ -117,7 +117,7 @@ func runNoteAdd(cmd *cobra.Command, _ []string) error {
 
 	noteID, err := trace.GenerateNoteID()
 	if err != nil {
-		return fmt.Errorf("failed to generate note ID: %w", err)
+		return errors.WrapInternal("failed to generate note ID", err)
 	}
 
 	note := trace.AnalystNote{

internal/errors/errors.go

@@ -546,6 +546,19 @@ func WrapAnalysisTruncated(phase, reason string) error {
 	}
 }
 
+// WrapInternal wraps an unexpected internal failure (e.g. ID generation,
+// unexpected I/O) that does not belong to any more specific sentinel family
+// [Issue #762]. Using this instead of returning a bare fmt.Errorf keeps every
+// command-facing failure a stable ErstError with a consistent code and exit
+// bucket rather than surfacing as ErstUnknown to automation.
+func WrapInternal(msg string, err error) error {
+	return &ErstError{
+		Code:    ErstInternalError,
+		Message: fmt.Sprintf("%s: %v", msg, err),
+		OrigErr: err,
+	}
+}
+
 const (
 	// RPC origin
 	CodeRPCConnectionFailed  ErstErrorCode = "RPC_CONNECTION_FAILED"

internal/errors/glassbox_error_code.go

@@ -72,6 +72,15 @@ const (
 	// or a broken hash chain — that would not be caught by per-file signature
 	// verification alone.
 	ErstAuditDirPolicyViolation ErstErrorCode = "AUDIT_DIR_POLICY_VIOLATION"
+
+	// Internal [Issue #762]
+	// ErstInternalError is returned when a command hits an unexpected
+	// internal failure (e.g. ID generation, unexpected I/O) that does not
+	// belong to any more specific sentinel family. Using this code instead of
+	// returning a bare, unwrapped error keeps every command-facing failure a
+	// stable ErstError so automation can rely on a consistent code and exit
+	// bucket rather than falling back to ErstUnknown.
+	ErstInternalError ErstErrorCode = "INTERNAL_ERROR"
 )
 
 // ErstError wraps an error with a standardized code and preserves the original error string.

scripts/verify-release.sh

@@ -44,42 +44,118 @@ for bin in "${EXPECTED[@]}"; do
   fi
 done
 
-# ── 2. Native binary executes ─────────────────────────────────────────────────
+# ── 2. Per-artifact smoke tests ───────────────────────────────────────────────
+#
+# Every supported artifact is exercised, not just the native one: startup and
+# a set of offline, network-free commands (version, help, demo, dry-run, and
+# JSON output) are run for each binary. A binary is only ever [FAIL]ed when a
+# runner capable of executing it was actually available; when no runner
+# exists for a given artifact's platform, its commands are reported as
+# [SKIP-UNAVAILABLE] so a missing emulator/runner is never conflated with a
+# real regression. Metadata (artifact, platform, command, exit code, and a
+# short output excerpt) is recorded in SMOKE_LOG for the release record.
 echo ""
-echo "2. Smoke-testing native binary..."
-OS="$(uname -s | tr '[:upper:]' '[:lower:]')"
-ARCH="$(uname -m)"
-case "${ARCH}" in
-  x86_64)  ARCH="amd64" ;;
-  aarch64|arm64) ARCH="arm64" ;;
-  *) ARCH="amd64" ;;
+echo "2. Smoke-testing every supported binary..."
+
+HOST_OS="$(uname -s | tr '[:upper:]' '[:lower:]')"
+HOST_ARCH="$(uname -m)"
+case "${HOST_ARCH}" in
+  x86_64)  HOST_ARCH="amd64" ;;
+  aarch64|arm64) HOST_ARCH="arm64" ;;
+  *) HOST_ARCH="amd64" ;;
 esac
 
-NATIVE_BIN="${DIST_DIR}/glassbox-${OS}-${ARCH}"
-if [ "${OS}" = "windows" ]; then
-  NATIVE_BIN="${NATIVE_BIN}.exe"
-fi
+SMOKE_LOG="${DIST_DIR}/smoke-results.log"
+: > "${SMOKE_LOG}" 2>/dev/null || SMOKE_LOG="/dev/null"
 
-if [ -f "${NATIVE_BIN}" ]; then
-  chmod +x "${NATIVE_BIN}"
-  if output=$("${NATIVE_BIN}" --version 2>&1 || "${NATIVE_BIN}" version 2>&1 || true); then
-    if [ -n "${output}" ]; then
-      pass "native binary executed: ${output}"
-    else
-      # Some CLIs exit 0 with no output for --version; try --help
-      help_output=$("${NATIVE_BIN}" --help 2>&1 | head -1 || true)
-      if [ -n "${help_output}" ]; then
-        pass "native binary executed (--help): ${help_output}"
-      else
-        fail "native binary produced no output"
-      fi
+# runner_for_artifact prints, on stdout, the command prefix (possibly via an
+# emulator) needed to execute a given binary on this host, or nothing if no
+# runner is available. artifact_os/artifact_arch describe the target.
+runner_for_artifact() {
+  artifact_os="$1"
+  artifact_arch="$2"
+  bin_path="$3"
+
+  if [ "${artifact_os}" = "${HOST_OS}" ] && [ "${artifact_arch}" = "${HOST_ARCH}" ]; then
+    printf '%s' "${bin_path}"
+    return 0
+  fi
+
+  if [ "${artifact_os}" = "windows" ] && command -v wine >/dev/null 2>&1; then
+    printf 'wine %s' "${bin_path}"
+    return 0
+  fi
+
+  if [ "${artifact_os}" = "linux" ] && [ "${HOST_OS}" = "linux" ]; then
+    case "${artifact_arch}" in
+      amd64) emu=qemu-x86_64-static ;;
+      arm64) emu=qemu-aarch64-static ;;
+      *) emu="" ;;
+    esac
+    if [ -n "${emu}" ] && command -v "${emu}" >/dev/null 2>&1; then
+      printf '%s %s' "${emu}" "${bin_path}"
+      return 0
     fi
-  else
-    fail "native binary failed to execute"
   fi
-else
-  echo "  [SKIP] native binary not found for ${OS}/${ARCH} (cross-compiled only)"
-fi
+
+  return 1
+}
+
+# run_smoke_command executes one offline command against one artifact and
+# records PASS/FAIL/SKIP-UNAVAILABLE. It never requires network access,
+# credentials, or a deployed contract — every command below is a local,
+# read-only or dry-run operation.
+run_smoke_command() {
+  artifact="$1"; shift
+  label="$1"; shift
+  # remaining args: the command to run
+
+  set +e
+  cmd_output=$("$@" 2>&1)
+  cmd_exit=$?
+  set -e
+
+  excerpt=$(printf '%s' "${cmd_output}" | head -1 | cut -c1-120)
+  printf 'artifact=%s command=%s exit=%d output=%q\n' "${artifact}" "${label}" "${cmd_exit}" "${excerpt}" >> "${SMOKE_LOG}"
+
+  if [ "${cmd_exit}" -eq 0 ] && [ -n "${cmd_output}" ]; then
+    pass "${artifact} ${label}: ${excerpt}"
+    return 0
+  fi
+  fail "${artifact} ${label} failed (exit ${cmd_exit}): ${excerpt}"
+  return 1
+}
+
+for bin in "${EXPECTED[@]}"; do
+  bin_path="${DIST_DIR}/${bin}"
+  if [ ! -f "${bin_path}" ]; then
+    # Already reported missing in step 1; nothing more to smoke-test.
+    continue
+  fi
+
+  # Derive platform from the artifact filename glassbox-<os>-<arch>[.exe].
+  rest="${bin#glassbox-}"
+  rest="${rest%.exe}"
+  artifact_os="${rest%-*}"
+  artifact_arch="${rest##*-}"
+
+  chmod +x "${bin_path}" 2>/dev/null || true
+
+  if ! runner=$(runner_for_artifact "${artifact_os}" "${artifact_arch}" "${bin_path}"); then
+    echo "  [SKIP-UNAVAILABLE] ${bin}: no runner/emulator on this host for ${artifact_os}/${artifact_arch}"
+    printf 'artifact=%s command=* exit=SKIP-UNAVAILABLE reason=%q\n' "${bin}" "no runner for ${artifact_os}/${artifact_arch}" >> "${SMOKE_LOG}"
+    continue
+  fi
+
+  # shellcheck disable=SC2206
+  runner_args=(${runner})
+
+  run_smoke_command "${bin}" "version"  "${runner_args[@]}" --version || true
+  run_smoke_command "${bin}" "help"     "${runner_args[@]}" --help || true
+  run_smoke_command "${bin}" "demo"     "${runner_args[@]}" debug --demo || true
+  run_smoke_command "${bin}" "dry-run"  "${runner_args[@]}" debug --dry-run --help || true
+  run_smoke_command "${bin}" "json"     "${runner_args[@]}" version --json || true
+done
 
 # ── 3. Checksums verify ───────────────────────────────────────────────────────
 echo ""