fix(onboarding): address trail 742 review findings

ace2530·

peyton-alt·15h ago·5 files·+148 added/-25 removed

• Defer ordering: enable's logging Close was registered after the bootstrap-finalize defer, so (LIFO) the log file closed before the deferred connect ladder ran and its warnings were silently dropped. Logging now initializes inside the bootstrap block, before the finalize defer (extracted initEnableLogging so callers control defer order).
• Probe cache: createAndAwaitMirror wrote through right after CreateMirror, so a clone that failed during the awaited poll still left a green checklist for the cache TTL. Write through only on confirmed-ready (or on --no-wait, where there is genuinely nothing more to know).
• status --json: setup rungs now carry detail and hint alongside state — the remediation command isn't derivable from the state name, per the Agent-Safe CLI Fallbacks convention.

Trail-Finding: 019f6d3c-1848-7aa7-918d-c6516a50fc23 Trail-Finding: 019f6d07-41fa-71d5-91d9-26ef8755761d Trail-Finding: 019f6cd3-b47c Co-Authored-By: Claude Fable 5 noreply@anthropic.com

Sessions

01KXR7HPN0FAFCHCPKB4MHM2SWView transcript

Changes

5

• cmd/entire/cli

• Mrepo_mirror.go+15/-4

• Mrepo_mirror_test.go+67

• Msetup.go+31/-8

• Mstatus.go+24/-6

• Mstatus_test.go+11/-7

445 unmodified lines

445 unmodified lines

healMirrorProbeCache(owner, repo)
        return outcome, nil
    }
    healMirrorProbeCache(owner, repo)
    if noWait {
        // Placement registered, clone continuing server-side — nothing more
        // to learn now, so record the serving placement.
        healMirrorProbeCache(owner, repo)
        return outcome, nil
    }
    status, werr := awaitMirrorReady(ctx, c, created.MirrorId, timeout, onStatus)
    outcome.status = status
    outcome.polled = true
    // Write through only on a confirmed-ready clone: caching at create time
    // would keep the checklist green for the cache TTL even when the clone
    // failed moments later. A timeout leaves the cache alone — the next live
    // probe reads the truth.
    if werr == nil && status == coreapi.MirrorStatusReady {
        healMirrorProbeCache(owner, repo)
    }
    return outcome, werr
}

// healMirrorProbeCache records a serving placement in the onboarding probe
// cache — the setup checklist's own hint is `entire repo mirror create
// <slug>`, and a cached "not mirrored" must not survive the prescribed
// remediation. A suspended placement never serves, so callers write through
// only after every suspension signal is checked (CreateMirror's Suspended
// flag, plus the GetMirror read for existing empty-upstream placements).
// remediation. Callers write through only once the placement's outcome is
// settled enough to trust for a TTL: after every suspension signal is
// checked (CreateMirror's Suspended flag, plus the GetMirror read for
// existing empty-upstream placements), and — when a clone is awaited — only
// on a confirmed-ready status, never before a clone that may still fail.
func healMirrorProbeCache(owner, repo string) {
    slugOwner, slugRepo := githubSlug(owner, repo)
    defaultMirrorProbeCache().put(mirrorProbeKey(slugOwner+"/"+slugRepo), mirrorProbeResult{Mirrored: true}, time.Now())
}

Mcmd/entire/cli/repo_mirror.go+15/-4

290 unmodified lines

290 unmodified lines

}) }

// A clone that fails after registration must not leave a green probe-cache // entry — the checklist would show ✓ for the cache TTL while the mirror never // became usable. A confirmed-ready clone and a --no-wait registration (where // there is genuinely nothing more to know yet) still write through. // Not parallel: redirects the probe cache and shortens the poll interval. func TestCreateAndAwaitMirror_CacheWaitsForCloneOutcome(t *testing.T) { t.Setenv("XDG_CACHE_HOME", t.TempDir()) prev := mirrorPollInterval mirrorPollInterval = time.Millisecond t.Cleanup(func() { mirrorPollInterval = prev }) ctx := t.Context()

serve := func(t *testing.T, status coreapi.MirrorStatus) *coreapi.Client { t.Helper() created := &coreapi.CreatedMirror{Created: true, MirrorId: "m1"} srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") switch { case r.Method == http.MethodPost && r.URL.Path == mirrorsAPIPath: w.WriteHeader(http.StatusCreated) if err := printJSON(w, created); err != nil { t.Errorf("encode created response: %v", err) } case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/api/v1/mirrors/"): m := &coreapi.Mirror{} m.Status = coreapi.NewOptMirrorStatus(status) if err := printJSON(w, m); err != nil { t.Errorf("encode mirror response: %v", err) } default: t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) w.WriteHeader(http.StatusNotFound) } })) t.Cleanup(srv.Close) c, err := coreapi.NewWithBearer(srv.URL, "tok") require.NoError(t, err) return c }

t.Run("failed clone is not cached as mirrored", func(t *testing.T) { c := serve(t, coreapi.MirrorStatusFailed) _, err := createAndAwaitMirror(ctx, c, "fail", "r", "c", false, time.Second, nil, nil) require.ErrorIs(t, err, errMirrorCloneFailed) _, _, ok := defaultMirrorProbeCache().get(mirrorProbeKey("fail/r"), time.Now()) require.False(t, ok, "a failed clone must not write through to the probe cache") })

t.Run("ready clone is cached", func(t *testing.T) { c := serve(t, coreapi.MirrorStatusReady) _, err := createAndAwaitMirror(ctx, c, "ok2", "r", "c", false, time.Second, nil, nil) require.NoError(t, err) probe, _, ok := defaultMirrorProbeCache().get(mirrorProbeKey("ok2/r"), time.Now()) require.True(t, ok, "a ready clone should be cached") require.True(t, probe.Mirrored) })

t.Run("no-wait registration is cached", func(t *testing.T) { c := serve(t, coreapi.MirrorStatusReady) _, err := createAndAwaitMirror(ctx, c, "nw", "r", "c", true, time.Second, nil, nil) require.NoError(t, err) _, _, ok := defaultMirrorProbeCache().get(mirrorProbeKey("nw/r"), time.Now()) require.True(t, ok, "a no-wait registration should be cached") }) }

func countEq(xs []string, want string) int { n := 0 for _, x := range xs {

Mcmd/entire/cli/repo_mirror_test.go+67

844 unmodified lines

844 unmodified lines

// the initial commit captures the .entire/, .claude/, hooks, and // settings files that setup writes. var bootstrap *bootstrapState loggingReady := false if _, err := paths.WorktreeRoot(ctx); err != nil { bootstrapOpts.Yes = opts.Yes state, bootstrapErr := runGitHubBootstrapInit(ctx, cmd.OutOrStdout(), cmd.ErrOrStderr(), bootstrapOpts)

// Visual separator between bootstrap init and agent setup.
printBootstrapSection(cmd.OutOrStdout(), "Enabling Entire")
// Logging must initialize BEFORE the finalize defer below is
// registered: defers run LIFO, so Close (registered here,
// first) runs after the deferred ladder — whose probe/offer
// warnings would otherwise be written to a closed log file
// and silently dropped. Safe at this point: phase 1 just
// created the repo, so Init can't leave a stray .entire/ in
// a folder whose bootstrap was declined.
defer initEnableLogging(ctx)()
loggingReady = true
// On the way out (if setup succeeded), create the initial
// commit and push to the GitHub repo. If setup returned an
// error, skip the finalize — the user can fix the issue and
// retry.

// Operational logging (ladder probes, offer failures) belongs in
// .entire/logs — without Init the logging package falls back to
// slog's stderr default and WARN lines leak verbatim into
// enable's output. After the bootstrap block, so a declined
// bootstrap doesn't leave a stray .entire/ in a non-repo folder.
logging.SetLogLevelGetter(GetLogLevel)
if lerr := logging.Init(ctx, ""); lerr == nil {
    defer logging.Close()
}

// After the bootstrap block, so a declined bootstrap doesn't
// leave a stray .entire/ in a non-repo folder; the bootstrap
// path initialized logging inside the block instead (before its
// finalize defer — see the LIFO note there).
if !loggingReady {
    defer initEnableLogging(ctx)()
}

if err := validateSetupFlags(opts.UseLocalSettings, opts.UseProjectSettings); err != nil {

...

}

Mcmd/entire/cli/setup.go+31/-8

602 unmodified lines

602 unmodified lines

// success path (mirrors writeAgentHelpHint, which only renders when set up).
    AgentHelp string `json:"agent_help,omitempty"`
    // Setup maps each onboarding rung (hooks, auth, mirror, import) to its
    // state: done, missing, blocked, unknown, or not_applicable. Present only
    // when enabled — same gate as the human checklist.
    Setup map[string]string `json:"setup,omitempty"`
    Error string            `json:"error,omitempty"`
    // state plus the same detail and remediation command the human checklist
    // shows — the state name alone isn't actionable for a non-interactive
    // agent (Agent-Safe CLI Fallbacks). Present only when enabled — same gate
    // as the human checklist.
    Setup map[string]setupRungJSON `json:"setup,omitempty"`
    Error string                   `json:"error,omitempty"`
}

// setupRungJSON is one onboarding rung in `entire status --json`.
type setupRungJSON struct {
    // State: done, missing, blocked, unknown, or not_applicable.
    State string `json:"state"`
    // Detail mirrors the human checklist row's annotation, e.g.
    // "2 claude-code sessions found, not imported".
    Detail string `json:"detail,omitempty"`
    // Hint is the remediation command for this rung, e.g.
    // "entire repo mirror create github.com/owner/repo".
    Hint string `json:"hint,omitempty"`
}

type sessionBriefJSON struct {
...
}

Mcmd/entire/cli/status.go+24/-6

2110 unmodified lines

2110 unmodified lines

writeSettings(t, testSettingsEnabled) stubOnboardingResults(t, []onboarding.Result{ {Rung: onboarding.Rung{Key: onboarding.KeyHooks, Title: "Agent hooks"}, Check: onboarding.Check{State: onboarding.StateDone}}, {Rung: onboarding.Rung{Key: onboarding.KeyAuth, Title: "Logged in"}, Check: onboarding.Check{State: onboarding.StateMissing}}, {Rung: onboarding.Rung{Key: onboarding.KeyMirror, Title: "Repo mirrored"}, Check: onboarding.Check{State: onboarding.StateBlocked}}, {Rung: onboarding.Rung{Key: onboarding.KeyAuth, Title: "Logged in"}, Check: onboarding.Check{State: onboarding.StateMissing, Hint: "entire auth login"}}, {Rung: onboarding.Rung{Key: onboarding.KeyMirror, Title: "Repo mirrored"}, Check: onboarding.Check{State: onboarding.StateBlocked, Detail: "needs login", Hint: "entire auth login"}}, })

var stdout bytes.Buffer

var parsed struct { Setup map[string]string json:"setup" Setup map[string]setupRungJSON json:"setup" } if err := json.Unmarshal(stdout.Bytes(), &parsed); err != nil { t.Fatalf("unmarshal status JSON: %v\noutput: %s", err, stdout.String()) } want := map[string]string{"hooks": "done", "auth": "missing", "mirror": "blocked"}; for key, state := range want { if parsed.Setup[key] != state { t.Errorf("setup[%q] = %q, want %q", key, parsed.Setup[key], state) } } want := map[string]setupRungJSON{ "hooks": {State: "done"}, "auth": {State: "missing", Hint: "entire auth login"}, "mirror": {State: "blocked", Detail: "needs login", Hint: "entire auth login"}, }; for key, rung := range want { if parsed.Setup[key] != rung { t.Errorf("setup[%q] = %+v, want %+v", key, parsed.Setup[key], rung) } } }

Mcmd/entire/cli/status_test.go+11/-7