Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions internal/cli/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,5 +135,12 @@ func runSelfUpdate(out io.Writer, opts install.UpdateOptions) error {
fmt.Fprintf(out, " daemon: %s\n", result.InstallResult.DaemonVersion)
}
}
// #5907 FIX4: surface the auto-reindex-on-upgrade the engine has already
// (loop-guarded) enqueued, so it reads as "reindexing after upgrade"
// rather than a silent multi-minute stall right after `grafel update`
// returns. Report-only — nothing here triggers or duplicates the reindex.
if result.ReposNeedingReindex > 0 {
fmt.Fprintf(out, " reindex: %d repo(s) reindexing after upgrade\n", result.ReposNeedingReindex)
}
return nil
}
62 changes: 61 additions & 1 deletion internal/dashboard/handlers_index_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ package dashboard
// "repos": [
// { "repo_slug": "<slug>", "indexing": <bool>, "enhancing": <bool>,
// "entities": <int64>, "relationships": <int64>,
// "graph_fb_mtime": <int64-ns> }
// "graph_fb_mtime": <int64-ns>,
// "reindex_required": <bool>, "reindex_reason": <string>,
// "status": "<idle|indexing|enhancing|reindex-required|reindexing-after-upgrade>" }
// ]
// }
// }
Expand All @@ -35,6 +37,18 @@ package dashboard
// exactly that "unknown" case. Missing/stale engine-liveness similarly
// degrades to a zero engine block rather than failing the whole response.
// This endpoint is read-only: no writes, no index trigger.
//
// #5907 SURFACING slice: before this change, ReindexRequired/ReindexReason
// (internal/statusfile.File, set by the engine's FIX2 auto-reindex-on-upgrade
// arm — internal/daemon/stale_reindex.go) were dropped when building
// indexStatusRepo, so a repo whose on-disk graph.fb was stamped by an older
// grafel build read as idle to the web dashboard — a silent multi-minute
// stall from the frontend's point of view, even though the engine had
// already loop-guard-enqueued a reindex. Those fields are now surfaced
// verbatim, plus a derived `status` string (see deriveIndexStatus) so the
// frontend can render a "reindexing after upgrade" badge instead of nothing
// at all. This mirrors internal/cli/statusline.go's shell-side
// `⟲ reindex required` rendering of the same statusfile fields.

import (
"net/http"
Expand All @@ -56,6 +70,49 @@ type indexStatusRepo struct {
Entities int64 `json:"entities"`
Relationships int64 `json:"relationships"`
GraphFBMtime int64 `json:"graph_fb_mtime"`

// ReindexRequired/ReindexReason mirror internal/statusfile.File's fields
// of the same name (#5907 FIX1/FIX2): true when the on-disk graph.fb this
// repo is CURRENTLY SERVING was written by an older grafel build than
// this engine's fbversion.Version supports. The engine has already
// loop-guard-enqueued a reindex by the time this is observed true — it is
// never a "please click something" prompt, only a visibility signal.
ReindexRequired bool `json:"reindex_required"`
ReindexReason string `json:"reindex_reason,omitempty"`

// Status is a derived, frontend-friendly single-word summary of the row
// (see deriveIndexStatus), so the dashboard can render one badge instead
// of re-deriving the same precedence from four booleans client-side.
Status string `json:"status"`
}

// deriveIndexStatus collapses a repo's raw statusfile flags into a single
// human-facing status string, in priority order:
//
// 1. "reindexing-after-upgrade" — ReindexRequired AND the auto-enqueued
// reindex is actively running (Indexing=true). This is the state #5907
// exists to surface: without it, a post-upgrade reindex reads as a
// generic "indexing" row indistinguishable from a normal reindex.
// 2. "reindex-required" — ReindexRequired but the auto-enqueued reindex
// hasn't started running yet (still queued behind other work, or the
// engine hasn't drained the request yet). Before this slice, THIS was
// the silent-stall window: the repo looked idle from the dashboard.
// 3. "indexing" — the ordinary extraction-phase state.
// 4. "enhancing" — queryable, background enrichment tail still running.
// 5. "idle" — none of the above.
func deriveIndexStatus(reindexRequired, indexing, enhancing bool) string {
switch {
case reindexRequired && indexing:
return "reindexing-after-upgrade"
case reindexRequired:
return "reindex-required"
case indexing:
return "indexing"
case enhancing:
return "enhancing"
default:
return "idle"
}
}

// indexStatusReply is the payload for GET /api/v2/groups/{group}/index-status.
Expand Down Expand Up @@ -98,7 +155,10 @@ func (s *Server) handleV2IndexStatus(w http.ResponseWriter, r *http.Request) {
row.Entities = f.Entities
row.Relationships = f.Relationships
row.GraphFBMtime = f.GraphFBMtime
row.ReindexRequired = f.ReindexRequired
row.ReindexReason = f.ReindexReason
}
row.Status = deriveIndexStatus(row.ReindexRequired, row.Indexing, row.Enhancing)
reply.Repos = append(reply.Repos, row)
}

Expand Down
93 changes: 87 additions & 6 deletions internal/dashboard/handlers_index_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,23 @@ func writeIndexStatusFixture(t *testing.T, repoPath string, indexing, enhancing
}
}

// writeIndexStatusReindexFixture writes a per-repo statusfile.File with
// ReindexRequired/ReindexReason set (#5907 FIX1/FIX2's fields), optionally
// while an auto-enqueued reindex is actively running (indexing=true).
func writeIndexStatusReindexFixture(t *testing.T, repoPath string, indexing bool, reason string) {
t.Helper()
f := &statusfile.File{
RepoPath: repoPath,
HeartbeatAt: time.Now().UTC(),
Indexing: indexing,
ReindexRequired: true,
ReindexReason: reason,
}
if err := statusfile.Write(repoPath, f); err != nil {
t.Fatalf("write reindex status fixture for %s: %v", repoPath, err)
}
}

// writeEngineLivenessFixture writes the engine-global liveness sidecar with
// the given CPU%/RSS, using the SAME (daemon.DefaultLayout + EngineLivenessStatusKey)
// derivation the production writer/reader use, so the handler under test
Expand Down Expand Up @@ -112,12 +129,15 @@ type indexStatusReplyWire struct {
RSSMB int64 `json:"rss_mb"`
} `json:"engine"`
Repos []struct {
RepoSlug string `json:"repo_slug"`
Indexing bool `json:"indexing"`
Enhancing bool `json:"enhancing"`
Entities int64 `json:"entities"`
Relationships int64 `json:"relationships"`
GraphFBMtime int64 `json:"graph_fb_mtime"`
RepoSlug string `json:"repo_slug"`
Indexing bool `json:"indexing"`
Enhancing bool `json:"enhancing"`
Entities int64 `json:"entities"`
Relationships int64 `json:"relationships"`
GraphFBMtime int64 `json:"graph_fb_mtime"`
ReindexRequired bool `json:"reindex_required"`
ReindexReason string `json:"reindex_reason"`
Status string `json:"status"`
} `json:"repos"`
}

Expand Down Expand Up @@ -213,6 +233,67 @@ func TestIndexStatusEndpoint_MissingStatusFileYieldsZeroRow(t *testing.T) {
if r.Indexing || r.Enhancing || r.Entities != 0 || r.Relationships != 0 || r.GraphFBMtime != 0 {
t.Errorf("untouched repo should be zero/false row, got %+v", r)
}
if r.ReindexRequired {
t.Errorf("untouched repo should have reindex_required=false, got true")
}
if r.Status != "idle" {
t.Errorf("untouched repo status = %q, want %q", r.Status, "idle")
}
}

// TestIndexStatusEndpoint_ReindexRequired_Surfaced is the FIX3 (#5907) RED
// test: before this change, ReindexRequired/ReindexReason were dropped when
// building indexStatusRepo, so a repo whose graph.fb needed a reindex after a
// format upgrade read as an idle row to the web dashboard — a silent stall.
// This proves the fields are surfaced verbatim, plus the derived `status`:
// "reindexing-after-upgrade" once the auto-enqueued reindex is actively
// running, "reindex-required" while it's still queued.
func TestIndexStatusEndpoint_ReindexRequired_Surfaced(t *testing.T) {
paths := seedIndexStatusRegistry(t, "demo", "queued", "running")
writeIndexStatusReindexFixture(t, paths["queued"], false, "graph format v2 incompatible with v3 — reindex required")
writeIndexStatusReindexFixture(t, paths["running"], true, "graph format v2 incompatible with v3 — reindex required")

url, done := newIndexStatusServer(t)
defer done()

resp, err := http.Get(url + "/api/v2/groups/demo/index-status")
if err != nil {
t.Fatal(err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
t.Fatalf("status %d", resp.StatusCode)
}
var env indexStatusEnvelope
if err := json.NewDecoder(resp.Body).Decode(&env); err != nil {
t.Fatalf("decode: %v", err)
}
byBlug := map[string]int{}
for i, r := range env.Data.Repos {
byBlug[r.RepoSlug] = i
}

queued := env.Data.Repos[byBlug["queued"]]
if !queued.ReindexRequired {
t.Error("queued repo: expected reindex_required=true")
}
if queued.ReindexReason == "" {
t.Error("queued repo: expected a non-empty reindex_reason")
}
if queued.Status != "reindex-required" {
t.Errorf("queued repo status = %q, want %q", queued.Status, "reindex-required")
}

running := env.Data.Repos[byBlug["running"]]
if !running.ReindexRequired {
t.Error("running repo: expected reindex_required=true")
}
if !running.Indexing {
t.Error("running repo: expected indexing=true")
}
if running.Status != "reindexing-after-upgrade" {
t.Errorf("running repo status = %q, want %q", running.Status, "reindexing-after-upgrade")
}
}

// TestIndexStatusEndpoint_UnknownGroup404s ensures the endpoint is read-only
Expand Down
74 changes: 74 additions & 0 deletions internal/install/doctor.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// - Conventions per-file SHA manifests (Warning)
// - .gitignore contains /.grafel/ in tracked repos (Warning)
// - Stale staging directories older than 7 days (Info)
// - Repos whose on-disk graph needs a reindex after a format upgrade (Info)
//
// JSON schema is pinned at schema_version=1. Bump SchemaVersion when
// the shape of DoctorReport changes in a backward-incompatible way.
Expand All @@ -35,6 +36,7 @@ import (

"github.com/cajasmota/grafel/internal/daemon"
"github.com/cajasmota/grafel/internal/daemon/service"
"github.com/cajasmota/grafel/internal/graph"
"github.com/cajasmota/grafel/internal/install/mcpreg"
"github.com/cajasmota/grafel/internal/install/rulesfiles"
"github.com/cajasmota/grafel/internal/install/skilllink"
Expand Down Expand Up @@ -249,6 +251,15 @@ func RunDoctor(opts DoctorOptions) (*DoctorReport, error) {
report.Checks = append(report.Checks, *staleCheck)
}

// ── Check 9: Reindex required after a format upgrade (issue #5907 FIX4) ──
// Report-only: the engine's FIX2 (internal/daemon/stale_reindex.go) already
// auto-enqueues a loop-guarded reindex for a repo in this state, so doctor
// need not (and must not) trigger anything here — it only makes the
// otherwise-silent post-upgrade reindex visible in `grafel doctor`.
if reindexCheck := checkReindexRequired(opts); reindexCheck != nil {
report.Checks = append(report.Checks, *reindexCheck)
}

// Determine overall OK: all Critical checks must pass.
hasCriticalFailure := false
for _, c := range report.Checks {
Expand Down Expand Up @@ -816,6 +827,69 @@ func checkStaleStagingDirs(statePath string) *CheckResult {
return &cr
}

// checkReindexRequired scans every registered repo (across every grafel
// group) for an on-disk graph.fb written by an older grafel build than this
// binary supports (graph.ReindexRequiredReason, issue #5907 FIX1/FIX2).
//
// This is REPORT-ONLY. By the time a repo is observed in this state the
// engine's stale-format auto-reindex arm (internal/daemon/stale_reindex.go's
// loop-guarded maybeEnqueue) has already enqueued a reindex on its own — this
// check exists purely so that in-flight reindex is VISIBLE in `grafel doctor`
// instead of the repo silently looking idle/current for the several minutes
// the reindex takes. Doctor must never enqueue anything itself here: the
// engine already owns that decision, and a second enqueuer would risk racing
// or duplicating the loop-guard's own bookkeeping.
//
// Severity is Info, mirroring checkStaleStagingDirs: this is advisory
// visibility, not a broken install — the engine is already fixing it.
//
// Returns nil when no groups are registered or no repo currently needs a
// reindex (avoids adding a check with no content).
func checkReindexRequired(opts DoctorOptions) *CheckResult {
groupsFn := opts.groupsFn
if groupsFn == nil {
groupsFn = registry.Groups
}
loadGroupFn := opts.loadGroupFn
if loadGroupFn == nil {
loadGroupFn = registry.LoadGroupConfig
}

groups, err := groupsFn()
if err != nil || len(groups) == 0 {
return nil
}

var drift []string
for _, g := range groups {
cfg, lerr := loadGroupFn(g.ConfigPath)
if lerr != nil || cfg == nil {
continue
}
for _, repo := range cfg.Repos {
stateDir := daemon.StateDirForRepo(repo.Path)
if stateDir == "" {
continue
}
if required, reason := graph.ReindexRequiredReason(stateDir); required {
drift = append(drift, fmt.Sprintf("%s: %s", repo.Path, reason))
}
}
}

if len(drift) == 0 {
return nil
}

summary := fmt.Sprintf("%d repo(s) need reindex after a format upgrade", len(drift))
return &CheckResult{
Surface: "reindex-required",
OK: false,
Severity: SeverityInfo,
Drift: append([]string{summary}, drift...),
}
}

// ── Engine liveness + version skew (ADR-0024 PR5, epic #5729) ─────────────────

// engineLivenessDeps abstracts the I/O checkEngineLiveness needs so tests can
Expand Down
Loading
Loading