From ca1ce698f00dbbfc249be34358a2ee222f332e81 Mon Sep 17 00:00:00 2001 From: Boris Nagaev Date: Mon, 4 May 2026 01:23:05 -0500 Subject: [PATCH] cmd/loop: bless text-only session updates Add an env-gated bless mode to recorded-session replay so CLI-only text changes can refresh fixtures without a live recording pass. The replay still uses the recorded gRPC traffic, stdin, and environment, and it only rewrites stdout, stderr, and run_error after the command preserves the recorded success or failure shape. Keep the updater strict by refusing to bless sessions when replay leaves recorded gRPC events unconsumed, and add focused tests for the rewrite rules. Document the bless workflow next to the session fixtures, including the need for -count=1 so the Go test cache does not skip updates. --- cmd/loop/session_fixture_update_test.go | 618 ++++++++++++++++++++++++ cmd/loop/session_replay_test.go | 96 +++- cmd/loop/testdata/sessions/AGENTS.md | 12 + 3 files changed, 716 insertions(+), 10 deletions(-) create mode 100644 cmd/loop/session_fixture_update_test.go diff --git a/cmd/loop/session_fixture_update_test.go b/cmd/loop/session_fixture_update_test.go new file mode 100644 index 00000000..0c641815 --- /dev/null +++ b/cmd/loop/session_fixture_update_test.go @@ -0,0 +1,618 @@ +package main + +import ( + "bytes" + "encoding/json" + "fmt" + "os" + "path/filepath" + "runtime" + "slices" + "strconv" + "testing" + + "github.com/stretchr/testify/require" +) + +// updateRecordedSessionsEnvVar enables replay bless mode for text-only fixture +// updates. +const updateRecordedSessionsEnvVar = "LOOP_UPDATE_RECORDED_SESSIONS" + +// replayedSessionOutput captures the user-visible output produced by an +// offline session replay. +type replayedSessionOutput struct { + stdout string + stderr string + stdoutChunks []string + stderrChunks []string + runError *string +} + +// updateRecordedSessionsEnabled reports whether replay should bless text-only +// fixture updates. +func updateRecordedSessionsEnabled() (bool, error) { + raw, ok := os.LookupEnv(updateRecordedSessionsEnvVar) + if !ok { + return false, nil + } + + enabled, err := strconv.ParseBool(raw) + if err != nil { + return false, fmt.Errorf("invalid %s value %q", + updateRecordedSessionsEnvVar, raw) + } + + return enabled, nil +} + +// sessionFixturePath resolves a relative fixture path under +// cmd/loop/testdata/sessions to an absolute path. +func sessionFixturePath(rel string) (string, error) { + _, filename, _, ok := runtime.Caller(0) + if !ok { + return "", fmt.Errorf("locate session fixture dir") + } + + loopDir := filepath.Dir(filename) + + return filepath.Join( + loopDir, "testdata", "sessions", filepath.FromSlash(rel), + ), nil +} + +// loadSessionFilePath reads and decodes a session fixture from disk. +func loadSessionFilePath(path string) (sessionFile, error) { + blob, err := os.ReadFile(path) + if err != nil { + return sessionFile{}, err + } + + var fixture sessionFile + if err := json.Unmarshal(blob, &fixture); err != nil { + return sessionFile{}, err + } + + return fixture, nil +} + +// writeSessionFilePath writes a session fixture using the recorder's JSON +// formatting. +func writeSessionFilePath(path string, fixture sessionFile) error { + file, err := os.Create(path) + if err != nil { + return err + } + defer file.Close() + + encoder := json.NewEncoder(file) + encoder.SetIndent("", " ") + + return encoder.Encode(fixture) +} + +// maybeUpdateSessionFixture rewrites the text-only portions of a recorded +// session if the replay output changed. +func maybeUpdateSessionFixture(path string, fixture sessionFile, + output replayedSessionOutput) (bool, error) { + + updated, changed, err := rewriteSessionFixture(fixture, output) + if err != nil || !changed { + return changed, err + } + + if err := writeSessionFilePath(path, updated); err != nil { + return false, err + } + + return true, nil +} + +// rewriteSessionFixture rewrites stdout, stderr and run_error while leaving +// the rest of the recorded interaction unchanged. +func rewriteSessionFixture(fixture sessionFile, + output replayedSessionOutput) (sessionFile, bool, error) { + + updated := fixture + changed := false + + if !optionalTextEqual(updated.Metadata.RunError, output.runError) { + updated.Metadata.RunError = cloneOptionalString(output.runError) + changed = true + } + + var err error + updated.Events, changed, err = rewriteExitEventRunError( + updated.Events, output.runError, changed, + ) + if err != nil { + return sessionFile{}, false, err + } + + updated.Events, changed, err = rewriteTextEvents( + updated.Events, eventStdout, fixtureStdout(fixture), + output.stdout, output.stdoutChunks, changed, + ) + if err != nil { + return sessionFile{}, false, err + } + + updated.Events, changed, err = rewriteTextEvents( + updated.Events, eventStderr, fixtureStderr(fixture), + output.stderr, output.stderrChunks, changed, + ) + if err != nil { + return sessionFile{}, false, err + } + + return updated, changed, nil +} + +// rewriteExitEventRunError updates the exit payload to match the replayed run +// error. +func rewriteExitEventRunError(events []sessionEvent, runError *string, + changed bool) ([]sessionEvent, bool, error) { + + data, err := json.Marshal(exitPayload{ + RunError: cloneOptionalString(runError), + }) + if err != nil { + return nil, changed, err + } + + updated := append([]sessionEvent(nil), events...) + + for i, event := range slices.Backward(updated) { + if event.Kind != eventExit { + continue + } + + if bytes.Equal(event.Data, data) { + return updated, changed, nil + } + + updated[i].Data = data + + return updated, true, nil + } + + updated = append(updated, sessionEvent{ + Kind: eventExit, + Data: data, + }) + + return updated, true, nil +} + +// rewriteTextEvents updates one text stream when the normalized aggregate text +// changed. +func rewriteTextEvents(events []sessionEvent, kind, recorded, actual string, + chunks []string, changed bool) ([]sessionEvent, bool, error) { + + normalizedEqual := normalizeTimestamps(recorded) == + normalizeTimestamps(actual) + + sourceChunks := chunks + sourceCombined := actual + if normalizedEqual { + sourceChunks = fixtureTextChunksByKind(events, kind) + sourceCombined = recorded + } + + replacement, err := canonicalTextEvents( + events, kind, sourceChunks, sourceCombined, + ) + if err != nil { + return nil, changed, err + } + + if normalizedEqual && textEventsMatch(events, kind, replacement) { + return events, changed, nil + } + + updated, err := replaceTextEvents(events, kind, replacement) + if err != nil { + return nil, changed, err + } + + return updated, true, nil +} + +// canonicalTextEvents builds the canonical replacement events for one text +// stream. +func canonicalTextEvents(events []sessionEvent, kind string, chunks []string, + combined string) ([]sessionEvent, error) { + + indices := textEventIndices(events, kind) + replacement := replacementTextChunks(chunks, combined) + if len(indices) != len(replacement) { + replacement = replacementTextChunks(nil, combined) + } + + return buildReplacementTextEvents(events, indices, kind, replacement) +} + +// replaceTextEvents rewrites the recorded events for a text stream using the +// provided canonical replacement events. +func replaceTextEvents(events []sessionEvent, kind string, + replacements []sessionEvent) ([]sessionEvent, error) { + + indices := textEventIndices(events, kind) + if len(indices) == 0 && len(replacements) == 0 { + return append([]sessionEvent(nil), events...), nil + } + + if replacements == nil { + replacements = []sessionEvent{} + } + + return replaceTextEventsWithCanonical( + events, kind, indices, replacements, + ) +} + +// replaceTextEventsWithCanonical splices canonical replacement events into the +// event stream. +func replaceTextEventsWithCanonical(events []sessionEvent, kind string, + indices []int, replacements []sessionEvent) ([]sessionEvent, error) { + + if len(indices) == len(replacements) { + updated := append([]sessionEvent(nil), events...) + for i, idx := range indices { + updated[idx].Data = replacements[i].Data + } + + return updated, nil + } + + if len(indices) > 0 { + firstIdx := indices[0] + updated := make([]sessionEvent, 0, + len(events)-len(indices)+len(replacements)) + + inserted := false + for i, event := range events { + if event.Kind == kind { + if !inserted && i == firstIdx { + updated = append(updated, replacements...) + inserted = true + } + continue + } + + updated = append(updated, event) + } + + return updated, nil + } + + if len(replacements) == 0 { + return append([]sessionEvent(nil), events...), nil + } + + insertAt := len(events) + for i, event := range events { + if event.Kind == eventExit { + insertAt = i + break + } + } + + updated := make([]sessionEvent, 0, len(events)+len(replacements)) + updated = append(updated, events[:insertAt]...) + updated = append(updated, replacements...) + updated = append(updated, events[insertAt:]...) + + return updated, nil +} + +// textEventsMatch reports whether the existing events already use the canonical +// representation for a text stream. +func textEventsMatch(events []sessionEvent, kind string, + replacements []sessionEvent) bool { + + indices := textEventIndices(events, kind) + if len(indices) != len(replacements) { + return false + } + + for i, idx := range indices { + if events[idx].TimeMS != replacements[i].TimeMS { + return false + } + if !bytes.Equal(events[idx].Data, replacements[i].Data) { + return false + } + } + + return true +} + +// buildReplacementTextEvents creates replacement text events while reusing the +// closest recorded timestamps when possible. +func buildReplacementTextEvents(events []sessionEvent, indices []int, + kind string, chunks []string) ([]sessionEvent, error) { + + if len(chunks) == 0 { + return nil, nil + } + + replacements := make([]sessionEvent, 0, len(chunks)) + for i, chunk := range chunks { + data, err := json.Marshal(newTextPayload(chunk)) + if err != nil { + return nil, err + } + + replacements = append(replacements, sessionEvent{ + TimeMS: replacementEventTime(events, indices, i), + Kind: kind, + Data: data, + }) + } + + return replacements, nil +} + +// replacementEventTime picks a stable timestamp for a replacement text event. +func replacementEventTime(events []sessionEvent, indices []int, idx int) int64 { + if len(indices) == 0 { + return 0 + } + + if idx < len(indices) { + return events[indices[idx]].TimeMS + } + + return events[indices[len(indices)-1]].TimeMS +} + +// textEventIndices returns the indexes of text events with the given kind. +func textEventIndices(events []sessionEvent, kind string) []int { + var indices []int + for i, event := range events { + if event.Kind == kind { + indices = append(indices, i) + } + } + + return indices +} + +// replacementTextChunks returns the recorded write chunks to persist. If the +// hook callbacks did not observe any chunks, fall back to the aggregate text so +// the fixture still captures the visible output. +func replacementTextChunks(chunks []string, combined string) []string { + if len(chunks) > 0 { + return append([]string(nil), chunks...) + } + + if combined == "" { + return nil + } + + return []string{combined} +} + +// fixtureStdout returns the concatenated stdout text stored in a fixture. +func fixtureStdout(fixture sessionFile) string { + return fixtureTextByKind(fixture.Events, eventStdout) +} + +// fixtureStderr returns the concatenated stderr text stored in a fixture. +func fixtureStderr(fixture sessionFile) string { + return fixtureTextByKind(fixture.Events, eventStderr) +} + +// fixtureTextByKind concatenates all text payloads for a given event kind. +func fixtureTextByKind(events []sessionEvent, kind string) string { + var out bytes.Buffer + for _, text := range fixtureTextChunksByKind(events, kind) { + out.WriteString(text) + } + + return out.String() +} + +// fixtureTextChunksByKind returns the decoded text for each event of a given +// kind. +func fixtureTextChunksByKind(events []sessionEvent, kind string) []string { + var chunks []string + for _, event := range events { + if event.Kind != kind { + continue + } + + var payload textPayload + if err := json.Unmarshal(event.Data, &payload); err != nil { + continue + } + + chunks = append(chunks, payload.text()) + } + + return chunks +} + +// optionalTextEqual compares optional text values using the same timestamp +// normalization rules as replay assertions. +func optionalTextEqual(a, b *string) bool { + switch { + case a == nil && b == nil: + return true + + case a == nil || b == nil: + return false + } + + return normalizeTimestamps(*a) == normalizeTimestamps(*b) +} + +// cloneOptionalString copies an optional string. +func cloneOptionalString(value *string) *string { + if value == nil { + return nil + } + + cloned := *value + + return &cloned +} + +// TestUpdateRecordedSessionsEnabled verifies update-mode env parsing. +func TestUpdateRecordedSessionsEnabled(t *testing.T) { + if current, ok := os.LookupEnv(updateRecordedSessionsEnvVar); ok { + t.Setenv(updateRecordedSessionsEnvVar, current) + } else { + t.Setenv(updateRecordedSessionsEnvVar, "false") + } + _ = os.Unsetenv(updateRecordedSessionsEnvVar) + + enabled, err := updateRecordedSessionsEnabled() + require.NoError(t, err) + require.False(t, enabled) + + t.Setenv(updateRecordedSessionsEnvVar, "true") + enabled, err = updateRecordedSessionsEnabled() + require.NoError(t, err) + require.True(t, enabled) + + t.Setenv(updateRecordedSessionsEnvVar, "invalid") + enabled, err = updateRecordedSessionsEnabled() + require.False(t, enabled) + require.EqualError(t, err, + "invalid LOOP_UPDATE_RECORDED_SESSIONS value \"invalid\"") +} + +// TestRewriteSessionFixtureUpdatesMatchingChunks verifies that bless mode +// rewrites text and run errors in place when the write counts are unchanged. +func TestRewriteSessionFixtureUpdatesMatchingChunks(t *testing.T) { + oldErr := "old error" + newErr := "new error" + + fixture := sessionFile{ + Metadata: sessionMetadata{ + RunError: &oldErr, + }, + Events: []sessionEvent{ + textEvent(t, 5, eventStdout, "old stdout\n"), + textEvent(t, 6, eventStderr, "old stderr\n"), + exitEvent(t, 7, &oldErr), + }, + } + + updated, changed, err := rewriteSessionFixture(fixture, + replayedSessionOutput{ + stdout: "new stdout\n", + stderr: "new stderr\n", + stdoutChunks: []string{"new stdout\n"}, + stderrChunks: []string{"new stderr\n"}, + runError: &newErr, + }, + ) + require.NoError(t, err) + require.True(t, changed) + require.Equal(t, &newErr, updated.Metadata.RunError) + require.Equal(t, int64(5), updated.Events[0].TimeMS) + require.Equal(t, "new stdout\n", textEventText(t, updated.Events[0])) + require.Equal(t, int64(6), updated.Events[1].TimeMS) + require.Equal(t, "new stderr\n", textEventText(t, updated.Events[1])) + require.Equal(t, &newErr, exitEventRunError(t, updated.Events[2])) +} + +// TestRewriteSessionFixtureReplacesChangedChunkCount verifies that bless mode +// collapses mismatched write counts to one text event to keep fixture diffs +// small. +func TestRewriteSessionFixtureReplacesChangedChunkCount(t *testing.T) { + fixture := sessionFile{ + Events: []sessionEvent{ + textEvent(t, 10, eventStdout, "old "), + textEvent(t, 11, eventStdout, "text\n"), + exitEvent(t, 12, nil), + }, + } + + updated, changed, err := rewriteSessionFixture(fixture, + replayedSessionOutput{ + stdout: "new output\n", + stdoutChunks: []string{"new ", "output", "\n"}, + }, + ) + require.NoError(t, err) + require.True(t, changed) + require.Len(t, updated.Events, 2) + require.Equal(t, "new output\n", textEventText(t, updated.Events[0])) + require.Equal(t, int64(10), updated.Events[0].TimeMS) + require.Equal(t, eventExit, updated.Events[1].Kind) +} + +// TestRewriteSessionFixtureSkipsNormalizedTimestampNoise verifies that bless +// mode does not churn fixtures when only timestamp formatting differs. +func TestRewriteSessionFixtureSkipsNormalizedTimestampNoise(t *testing.T) { + fixture := sessionFile{ + Events: []sessionEvent{ + textEvent(t, 3, eventStdout, + "2026-05-04T10:00:00-05:00\n"), + exitEvent(t, 4, nil), + }, + } + + updated, changed, err := rewriteSessionFixture(fixture, + replayedSessionOutput{ + stdout: "2026-05-04T15:00:00Z\n", + stdoutChunks: []string{"2026-05-04T15:00:00Z\n"}, + }, + ) + require.NoError(t, err) + require.False(t, changed) + require.Equal(t, fixture, updated) +} + +// textEvent builds a text session event for tests. +func textEvent(t *testing.T, timeMS int64, kind, text string) sessionEvent { + t.Helper() + + data, err := json.Marshal(newTextPayload(text)) + require.NoError(t, err) + + return sessionEvent{ + TimeMS: timeMS, + Kind: kind, + Data: data, + } +} + +// exitEvent builds an exit session event for tests. +func exitEvent(t *testing.T, timeMS int64, runError *string) sessionEvent { + t.Helper() + + data, err := json.Marshal(exitPayload{ + RunError: cloneOptionalString(runError), + }) + require.NoError(t, err) + + return sessionEvent{ + TimeMS: timeMS, + Kind: eventExit, + Data: data, + } +} + +// textEventText decodes a text payload for assertions. +func textEventText(t *testing.T, event sessionEvent) string { + t.Helper() + + var payload textPayload + require.NoError(t, json.Unmarshal(event.Data, &payload)) + + return payload.text() +} + +// exitEventRunError decodes an exit payload for assertions. +func exitEventRunError(t *testing.T, event sessionEvent) *string { + t.Helper() + + var payload exitPayload + require.NoError(t, json.Unmarshal(event.Data, &payload)) + + return payload.RunError +} diff --git a/cmd/loop/session_replay_test.go b/cmd/loop/session_replay_test.go index c50c16b1..34fe55ed 100644 --- a/cmd/loop/session_replay_test.go +++ b/cmd/loop/session_replay_test.go @@ -636,6 +636,9 @@ func compareJSONWithContext(method, event string, idx int, actual []byte, // NOTE: Do not add t.Parallel() here; the replay harness mutates package-level // globals such as the active transport, clock, and JSON normalization mode. func TestRecordedSessions(t *testing.T) { + updateSessions, err := updateRecordedSessionsEnabled() + require.NoError(t, err) + restoreHelpDefaults := hookRecordedHelpDefaults() defer restoreHelpDefaults() @@ -670,6 +673,20 @@ func TestRecordedSessions(t *testing.T) { for _, path := range sessionFiles { t.Run(path, func(t *testing.T) { + var ( + fixture sessionFile + fixturePath string + ) + if updateSessions { + fixturePath, err = sessionFixturePath(path) + require.NoErrorf(t, err, + "resolve fixture path for %s", path) + + fixture, err = loadSessionFilePath(fixturePath) + require.NoErrorf(t, err, + "load fixture for update %s", path) + } + // Force deterministic JSON output for replay. prevDeterministic := forceDeterministicJSON forceDeterministicJSON = true @@ -683,14 +700,17 @@ func TestRecordedSessions(t *testing.T) { // Capture replay output for comparison. var ( - stdoutBuf bytes.Buffer - stderrBuf bytes.Buffer + stdoutBuf bytes.Buffer + stderrBuf bytes.Buffer + stdoutChunks []string + stderrChunks []string ) // Hook stdout for capture. stdoutUnhook, err := hookStdout( os.Stdout, nil, func(p []byte) { stdoutBuf.Write(p) + stdoutChunks = append(stdoutChunks, string(p)) }, ) require.NoErrorf(t, err, "hook stdout for %s", path) @@ -699,6 +719,7 @@ func TestRecordedSessions(t *testing.T) { stderrUnhook, err := hookStderr( os.Stderr, nil, func(p []byte) { stderrBuf.Write(p) + stderrChunks = append(stderrChunks, string(p)) }, ) require.NoErrorf(t, err, "hook stderr for %s", path) @@ -751,16 +772,41 @@ func TestRecordedSessions(t *testing.T) { t, stdinUnhook(), "unhook stdin for %s", path, ) - if replay.runError != nil { - require.Error(t, err, "expected run error") + // Validate the recorded error status matches the + // replay result. + actualRunError := errorString(err) + requireReplayOutcomeClass( + t, path, replay.runError, actualRunError, + ) - require.Equalf( - t, *replay.runError, err.Error(), - "run error mismatch for %s", path, - ) - } else { + if updateSessions { require.NoErrorf( - t, err, "command failed for %s", path, + t, replay.conn.assertFullyConsumed(), + "grpc replay incomplete for %s", path, + ) + + updated, updateErr := maybeUpdateSessionFixture( + fixturePath, fixture, replayedSessionOutput{ + stdout: stdoutBuf.String(), + stderr: stderrBuf.String(), + stdoutChunks: stdoutChunks, + stderrChunks: stderrChunks, + runError: actualRunError, + }, + ) + require.NoErrorf(t, updateErr, + "update fixture %s", path) + if updated { + t.Logf("updated %s", path) + } + + return + } + + if replay.runError != nil { + require.Equalf( + t, *replay.runError, *actualRunError, + "run error mismatch for %s", path, ) } @@ -819,6 +865,36 @@ func TestHookRecordedHelpDefaults(t *testing.T) { require.Equal(t, origMacaroon, macaroonPathFlag.DefaultText) } +// errorString converts an error to an optional string pointer. +func errorString(err error) *string { + if err == nil { + return nil + } + + msg := err.Error() + + return &msg +} + +// requireReplayOutcomeClass verifies that replay preserved the recorded +// success/failure shape even when bless mode is updating user-visible text. +func requireReplayOutcomeClass(t *testing.T, path string, expected, + actual *string) { + + t.Helper() + + switch { + case expected == nil && actual == nil: + return + + case expected == nil && actual != nil: + t.Fatalf("command failed for %s: %v", path, *actual) + + case expected != nil && actual == nil: + t.Fatalf("expected run error for %s", path) + } +} + // newRootCommandForReplay returns a root command clone with fresh flag state. func newRootCommandForReplay() *cli.Command { // Clone the root command tree to avoid shared flag state. diff --git a/cmd/loop/testdata/sessions/AGENTS.md b/cmd/loop/testdata/sessions/AGENTS.md index 8eb81618..d5d131ef 100644 --- a/cmd/loop/testdata/sessions/AGENTS.md +++ b/cmd/loop/testdata/sessions/AGENTS.md @@ -45,6 +45,18 @@ Base URL: `http://127.0.0.1:12345` - Session replay now clones the CLI command tree per run, so flag state (`IsSet`) does not leak between sessions. - Historical warning: earlier replays could have sticky flags across runs; if you see odd ordering-dependent failures, re-check that the replay uses the cloned command path. +## Bless mode for CLI text changes +- Use `LOOP_UPDATE_RECORDED_SESSIONS=true` to let `TestRecordedSessions` rewrite recorded `stdout`, `stderr`, and `run_error` values after a successful offline replay. +- Always run bless mode with `-count=1` so the Go test cache does not skip the update: + - `LOOP_UPDATE_RECORDED_SESSIONS=true go test ./cmd/loop -run TestRecordedSessions -count=1 -v` +- You can target a narrower subset with `-run`, for example: + - `LOOP_UPDATE_RECORDED_SESSIONS=true go test ./cmd/loop -run 'TestRecordedSessions/static-openchannel' -count=1 -v` +- Bless mode is intentionally narrow: + - it reuses the recorded gRPC stream, stdin, and env, + - it refuses to bless a session if the command changes from success to failure or vice versa, + - and it refuses to bless a session if replay no longer consumes the same recorded gRPC interaction. +- Use live recording, not bless mode, when a command’s behavior or RPC flow changed. + ## Session coverage map | Subdir | Commands / scenarios | | --- | --- |