From 4ad8525764dbfb0f7f03eae106b4436cc44040cc Mon Sep 17 00:00:00 2001 From: bigbes Date: Wed, 22 Jul 2026 18:39:02 +0300 Subject: [PATCH] feat(cmd): reindex a space when a push lands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The push notifier was a Phase 1 stub that logged 'reindex pending' because bleve did not exist yet. It does now, so a pushed document was readable but never searchable, and the reconciler reported permanent staleness. Pushes touching only proposal branches skip the rebuild: the index holds the approved revision, and making unreviewed text searchable is the same leak as serving it from the read plane. The index stamp is written last and only on success. Written earlier it would assert the index reflects a revision it does not — the exact staleness the reconciler exists to catch, and it would catch nothing. The hook server is now built after the surfaces, since the notifier reindexes through the same single-writer index they read from. --- cmd/specsrht/main.go | 73 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 60 insertions(+), 13 deletions(-) diff --git a/cmd/specsrht/main.go b/cmd/specsrht/main.go index 7b5678bfe9a33cee052aeaf1c464cad90b8b6b7c..d6031e5bb11480266e3cb1c4ab5a2332666ed07c 100644 --- a/cmd/specsrht/main.go +++ b/cmd/specsrht/main.go @@ -261,11 +261,19 @@ func run(log *slog.Logger) error { return err } + surf, err := newSurfaces(conf, cfg, svc, version) + if err != nil { + return err + } + defer surf.Close() + + // After the surfaces, because the push notifier reindexes through the same + // index they read from — one bleve writer, held here. hookSrv, err := hooks.NewServer(hooks.Options{ Backend: svc, Socket: hooks.SocketPath(cfg.Repos), Log: log, - OnPush: pushNotifier(log), + OnPush: pushNotifier(log, svc, surf.index), }) if err != nil { return err @@ -274,12 +282,6 @@ func run(log *slog.Logger) error { return err } - surf, err := newSurfaces(conf, cfg, svc, version) - if err != nil { - return err - } - defer surf.Close() - // server.New parses -b/-d/-m/-p and runs crypto.InitCrypto(conf), whose // two required keys validateConfig already checked, so it cannot fatal // here for a reason we have not already reported. @@ -520,16 +522,61 @@ func mountWeb(router chi.Router, _ ini.File, s *surfaces) { // index rev stamp are Phase 2's, because bleve and the stamp arrive together: // moving the stamp now, with no index behind it, would assert that the index // is current and remove the reconciler's only way of noticing that it is not. -func pushNotifier(log *slog.Logger) hooks.PushNotifier { - return func(_ context.Context, space core.SpaceRef, updates []hooks.RefUpdate) error { +func pushNotifier(log *slog.Logger, svc *service.Service, index *search.Index) hooks.PushNotifier { + return func(ctx context.Context, space core.SpaceRef, updates []hooks.RefUpdate) error { refs := make([]string, 0, len(updates)) for _, u := range updates { refs = append(refs, u.String()) } - log.Info("push landed; reindex pending", - "space", space.String(), - "refs", refs, - "note", "indexing lands in Phase 2; the reconciler reports the staleness until then") + + // A push that touches only proposal branches changes nothing the index + // holds: the index carries the approved revision, and proposal content + // is deliberately not searchable — surfacing unreviewed text in search + // is the same leak as serving it from the read plane. + approved := false + for _, u := range updates { + if !strings.HasPrefix(u.Ref, "refs/heads/"+core.ProposalPrefix) { + approved = true + break + } + } + if !approved { + log.Info("push landed; no reindex needed", "space", space.String(), "refs", refs) + return nil + } + + sp, err := svc.OpenSpace(ctx, space) + if err != nil { + return fmt.Errorf("open %s to reindex: %w", space, err) + } + rev, err := svc.ResolveRev(ctx, sp, service.ApprovedRev) + if err != nil { + return fmt.Errorf("resolve the approved head of %s: %w", space, err) + } + arc, bodies, err := svc.Archive(ctx, sp, service.ApprovedRev) + if err != nil { + return fmt.Errorf("read %s at %s: %w", space, rev, err) + } + docs, err := search.Extract(arc, bodies) + if err != nil { + return fmt.Errorf("project %s for indexing: %w", space, err) + } + stats, err := index.RebuildSpace(ctx, space, docs) + if err != nil { + return fmt.Errorf("reindex %s: %w", space, err) + } + + // The stamp goes last and only on success. Written earlier it would + // assert the index reflects a revision it does not, which is precisely + // the staleness the reconciler exists to detect — and it would detect + // nothing. + if _, err := svc.Store().SetIndexStamp(ctx, sp.ID, rev); err != nil { + return fmt.Errorf("stamp the index for %s at %s: %w", space, rev, err) + } + + log.Info("push landed; space reindexed", + "space", space.String(), "refs", refs, "rev", rev, + "indexed", stats.Indexed, "deleted", stats.Deleted, "took", stats.Took.String()) return nil } }