fix(#5991,#6013): reset reports an unconfirmed rebuild as a failure; status reports a corrupt graph as failed

[cajasmota]

Two release blockers in one class: the tool reported success or health when it wasn't true. grafel is consumed by AI agents that treat its output as ground truth, so a silently-wrong success is worse than a loud failure.

#5991 — grafel reset exited 0 on a rebuild that never ran

The reported mechanism was a rebuild cooldown. It was not. Split mode is the default (daemon.SplitModeEnabled(), internal/daemon/server.go:298 — true unless explicitly falsy). In split mode Service.Rebuild (service.go:657-683) writes a KindRebuild request file and returns nil with the zero RebuildReply unless WaitForCompletion is set, which reset did not set. The CLI printed Rebuilding group 'X'..., received {repos: nil, elapsed: 0, entities: 0, err: nil} instantly, took finishRebuild's len(repos)==0 branch, built an empty summaryParts, and printed nothing.

Both the .grafel/ wipe and the rebuild live engine-side in RebuildFunc, so when the engine never drains that request — engine down, crash-resume backoff, dead-lettered after maxRebuildAttempts — nothing happens at all. The CLI was structurally incapable of distinguishing that from success: byte-identical output, and the exit code carried no information in either case.

Fixed by opting reset into the existing #5790 WaitForCompletion contract on both call sites, so the daemon blocks on the engine's terminal ack and returns nil only on a real StatusOK. Failures are wrapped: reset: group "X" was NOT wiped or rebuilt (the previous graph is still on disk): <reason>.

The first fix did not reach the path users are on

Review reproduced #5991 on the fixed branch, which is the most useful thing in this PR.

The dashboard's progress handler (internal/dashboard/handlers_progress.go:246-262) matches only on RunToken and has no repo scoping, so it closes the whole group stream on the first per-repo terminal event. The CLI then fell into its !ok arm, waited 5 s, and returned the zero outcome with err == nil — so wrapResetFailure(wipe, group, nil) was a no-op and the fix never executed:

--- FAIL: TestReset_SSEStreamClosesEarly_DoesNotFalselySucceed (5.03s)
    reset returned (err=<nil>) before the daemon confirmed anything
    Rebuilding group 'mygroup'...
    repo-a: done  (7 entities)

And the first fix made that ordering normal: previously the RPC returned in milliseconds so resultCh was always ready first; WaitForCompletion deliberately blocks it for the whole rebuild, so for any group taking >5 s past its first repo, SSE-close-beats-RPC is the expected interleaving. The dashboard is embedded and started by default, so dashPort > 0 and the SSE branch is production — the poll path, which the original tests exercised, runs only when the dashboard is down.

runBrokerProgress now takes waitForCompletion. When a completion guarantee was requested the RPC is the only authority: sseCh = nil and continue, so the loop keeps selecting on resultCh and the 10 s heartbeat for as long as the rebuild takes. Callers that did not ask for one (rebuild, the wizard's indexGroupWithProgress) keep the historical 5 s give-up, pinned by TestRebuild_SSEGiveUpUnchanged so the guard binds in both directions.

--wait-timeout (BEHAVIOUR CHANGE)

Blocking on an engine ack means reset --quiet could otherwise hang for the daemon's full 2 h (rebuildWaitTimeout, and net/rpc over the UDS sets no client deadline). A CI invocation that used to return instantly could hang.

A flag rather than a default cap: a default cap would false-fail a legitimately long monorepo reset — the same confident-but-wrong report this issue is about, pointed the other way. Default stays unbounded; --wait-timeout 30m bounds the CLI's wait only. Applied via boundOutcome at the single channel every renderer consumes, so it cannot be live on one path and dead on another, and --quiet runs its RPC on a goroutine so the bound reaches it too. A malformed value is rejected, not ignored.

The give-up message is deliberately weaker than a confirmed failure, because the CLI stopped waiting rather than learning the outcome:

reset: group "X" — no completion confirmed within 30m: the daemon did not answer
in time; the rebuild may or may not have completed (check `grafel status`), and
--wait-timeout only bounds this command, it does not cancel the daemon

#6013 — grafel status reported a corrupt graph as healthy

Two swallows on the same call, not the one reported: an empty recover() at status_stats.go:149-155, and if err == nil && doc != nil { … } dropping the returned error with no else branch. Both verified reachable with real fixtures — a truncated graph.fb panics the flatbuffers accessors, an all-zero one returns a format-version error.

RepoStatus.GraphLoadError deliberately reuses the string representation internal/mcp already uses (LoadedRepo.loadErr) rather than inventing a type. Rendered as ⚠ graph FAILED to load: … — counts above are INCOMPLETE, with graphArtifactPresent keeping "no graph yet" distinct from "failed". The recover stays, so status remains non-fatal and still reports the other repos. The group TOTAL: line is now marked ⚠ INCOMPLETE — N repos could not be read.

Error text is scrubbed to basenames — a corrupt segment-set manifest leaked an absolute path twice in one message. Diagnosis survives: graph: resolve segment-set graph.3: graph: parse manifest in graph.3: invalid character 'n' ….

Verification

21 mutations across both rounds, all caught. Independently re-verified: restoring the 5 s SSE give-up fails both TestReset_SSEStreamClosesEarly_* at 5.01 s.

One test-quality problem found and fixed mid-review: a mutant originally hung the package rather than failing, because the wait-timeout tests called runResetCmd synchronously. A mutation that manifests only as a hang is not one the suite catches. runResetBounded (goroutine + bounded select) makes those fail in 10 s with an attributable message.

go build ./... && go vet ./... && gofmt -l . clean; internal/cli — the only package touched — passes -count=1 and -race, raw exit codes via rtk proxy. Revertability checked by execution: commit 1 alone builds and passes, and reverting commit 1 leaves commit 2 building and passing.

Known and disclosed

• grafel rebuild retains the same class of bug — fire-and-forget, empty reply, 5 s give-up, exit 0. Deliberately out of scope and pinned by two scope tests so it is an explicit decision, not an accident. Tracked as grafel rebuild can silently no-op in split mode (same shape as #5991) #6032.
• finishRebuild prints nothing on a successful split-mode reset (empty summaryParts), so success still reads as silence. Untouched.
• indexGroupWithProgress (wizard / group add) keeps the 5 s give-up; group add --index gets its honesty from indexGroup's own WaitForCompletion path (Split mode: 'group add --index' reports indexed:true off a fire-and-forget Rebuild ack (rebuild not actually done) #5790), not this one.
• The DialProgress-failure branch in runPollProgress has a wrapResetFailure call no test binds — it needs DialProgress to fail against a live socket. Genuinely rare, unlike the SSE branch.
• boundOutcome leaks the RPC goroutine on expiry; net/rpc offers no cancellation. Documented in code, reachable only when --wait-timeout is set and expires.
• case doc == nil in status is defensive and untested — not constructible through LoadGraphFromDir today.

Independently adversarially reviewed.

Closes #5991
Closes #6013