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
70 changes: 70 additions & 0 deletions cli/agent/codex/lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,20 @@ import (
"encoding/json"
"fmt"
"io"
"log/slog"
"os"
"time"

"github.com/GrayCodeAI/trace/cli/agent"
"github.com/GrayCodeAI/trace/cli/logging"
)

// Compile-time interface assertions.
var (
_ agent.HookSupport = (*CodexAgent)(nil)
_ agent.HookResponseWriter = (*CodexAgent)(nil)
_ agent.ContextInjector = (*CodexAgent)(nil)
_ agent.TranscriptPreparer = (*CodexAgent)(nil)
)

// WriteHookResponse outputs a JSON hook response to stdout.
Expand Down Expand Up @@ -183,3 +186,70 @@ func (c *CodexAgent) parseTurnEnd(stdin io.Reader) (*agent.Event, error) {
Timestamp: time.Now(),
}, nil
}

// PrepareTranscript waits for Codex's async rollout writes to settle before
// turn-end reads the file. Codex appends to the rollout JSONL from a
// background writer, so reading immediately after the stop hook can capture a
// truncated transcript that then gets condensed into a checkpoint snapshot.
//
// The rollout is append-only, so settle-on-stability (size held steady through
// a quiet window) is the completion proxy: any observed growth resets the
// window, so a transcript still being written with sub-second pauses keeps
// waiting up to maxWait, while a genuinely settled file returns well under it.
func (c *CodexAgent) PrepareTranscript(ctx context.Context, sessionRef string) error {
waitForRolloutSettle(ctx, sessionRef)
return nil
}

func waitForRolloutSettle(ctx context.Context, transcriptPath string) {
const (
maxWait = 3 * time.Second
pollInterval = 50 * time.Millisecond
quietWindow = 500 * time.Millisecond
staleThreshold = 2 * time.Minute
)

logCtx := logging.WithComponent(ctx, "agent.codex")

info, err := os.Stat(transcriptPath)
if err != nil {
// File doesn't exist (or is unreadable) — nothing to poll.
return
}
fileAge := time.Since(info.ModTime())
if fileAge > staleThreshold {
logging.Debug(
logCtx, "codex rollout is stale, skipping settle wait",
slog.Duration("file_age", fileAge),
)
return
}

deadline := time.Now().Add(maxWait)
lastSize := int64(-1)
var stableSince time.Time
for time.Now().Before(deadline) {
if err := ctx.Err(); err != nil {
return
}
if fi, statErr := os.Stat(transcriptPath); statErr == nil {
switch {
case fi.Size() != lastSize:
lastSize = fi.Size()
stableSince = time.Now()
case time.Since(stableSince) >= quietWindow:
logging.Debug(
logCtx, "codex rollout settled (size stable through quiet window), proceeding",
slog.Duration("quiet_window", quietWindow),
slog.Int64("size", fi.Size()),
)
return
}
}
time.Sleep(pollInterval)
}
logging.Warn(
logCtx, "codex rollout not settled within timeout, proceeding",
slog.Duration("timeout", maxWait),
)
}
128 changes: 128 additions & 0 deletions cli/agent/codex/settle_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
package codex

import (
"context"
"os"
"path/filepath"
"testing"
"time"
)

func TestWaitForRolloutSettle_StaleFile_ReturnsFast(t *testing.T) {
t.Parallel()

rollout := filepath.Join(t.TempDir(), "rollout.jsonl")
if err := os.WriteFile(rollout, []byte(`{"type":"session_meta","payload":{}}`+"\n"), 0o644); err != nil {
t.Fatalf("failed to write rollout: %v", err)
}
staleTime := time.Now().Add(-10 * time.Minute)
if err := os.Chtimes(rollout, staleTime, staleTime); err != nil {
t.Fatalf("failed to set mtime: %v", err)
}

start := time.Now()
waitForRolloutSettle(context.Background(), rollout)
elapsed := time.Since(start)

// Stale rollouts (agent no longer running) must not burn the 3s maxWait.
if elapsed > 500*time.Millisecond {
t.Errorf("expected fast return for stale rollout, but took %v", elapsed)
}
}

func TestWaitForRolloutSettle_StableFile_ReturnsFast(t *testing.T) {
t.Parallel()

// A recent rollout that has stopped growing (healthy turn-end: the final
// response_item and task_completed markers have been flushed). The wait
// must settle on size stability and return well under maxWait.
rollout := filepath.Join(t.TempDir(), "rollout.jsonl")
if err := os.WriteFile(rollout, []byte(`{"type":"response_item","payload":{"type":"message"}}`+"\n"), 0o644); err != nil {
t.Fatalf("failed to write rollout: %v", err)
}

start := time.Now()
waitForRolloutSettle(context.Background(), rollout)
elapsed := time.Since(start)

if elapsed > 1500*time.Millisecond {
t.Errorf("expected fast return for a stable recent rollout, but took %v", elapsed)
}
}

func TestWaitForRolloutSettle_GrowingFile_WaitsUntilSettled(t *testing.T) {
t.Parallel()

// A rollout still being appended to (async codex writer mid-turn) must NOT
// be declared settled while it grows; the wait returns only after the
// writes stop.
rollout := filepath.Join(t.TempDir(), "rollout.jsonl")
if err := os.WriteFile(rollout, []byte(`{"type":"event_msg","payload":{}}`+"\n"), 0o644); err != nil {
t.Fatalf("failed to write rollout: %v", err)
}

const growFor = 600 * time.Millisecond
stop := make(chan struct{})
go func() {
f, err := os.OpenFile(rollout, os.O_APPEND|os.O_WRONLY, 0o644)
if err != nil {
return
}
defer f.Close()
ticker := time.NewTicker(40 * time.Millisecond)
defer ticker.Stop()
deadline := time.Now().Add(growFor)
for {
select {
case <-stop:
return
case <-ticker.C:
if time.Now().After(deadline) {
return
}
if _, werr := f.WriteString(`{"type":"response_item","payload":{"type":"message"}}` + "\n"); werr != nil {
return
}
}
}
}()

start := time.Now()
waitForRolloutSettle(context.Background(), rollout)
elapsed := time.Since(start)
close(stop)

// Keep waiting while the file grows, but return once writes stop (bounded
// by the 3s cap).
if elapsed < 300*time.Millisecond {
t.Errorf("expected to keep waiting while rollout grew, returned after only %v", elapsed)
}
if elapsed > 3500*time.Millisecond {
t.Errorf("expected to return once settled/within cap, but took %v", elapsed)
}
}

func TestWaitForRolloutSettle_MissingFile_ReturnsFast(t *testing.T) {
t.Parallel()

// turn-end for a session whose rollout path is null or not yet created
// (e.g. --ephemeral mode) must not block.
start := time.Now()
waitForRolloutSettle(context.Background(), filepath.Join(t.TempDir(), "missing.jsonl"))
elapsed := time.Since(start)

if elapsed > 500*time.Millisecond {
t.Errorf("expected fast return for missing rollout, but took %v", elapsed)
}
}

func TestCodexAgent_PrepareTranscript_TurnEndContract(t *testing.T) {
t.Parallel()

// PrepareTranscript must always succeed (best-effort flush in a hook path:
// a failure would otherwise abort the agent's turn).
ag := &CodexAgent{}
if err := ag.PrepareTranscript(context.Background(), filepath.Join(t.TempDir(), "does-not-exist.jsonl")); err != nil {
t.Fatalf("PrepareTranscript must not error on missing transcript, got: %v", err)
}
}
31 changes: 23 additions & 8 deletions cli/strategy/condense_skip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"
"time"

"github.com/GrayCodeAI/trace/cli/checkpoint"
"github.com/GrayCodeAI/trace/cli/checkpoint/id"
"github.com/GrayCodeAI/trace/cli/session"
"github.com/GrayCodeAI/trace/redact"
Expand Down Expand Up @@ -114,11 +115,13 @@ func TestCondenseSession_DoesNotSkipWhenFilesTouchedButNoTranscript(t *testing.T
}

// Regression: a session whose tracked files don't overlap with this commit AND
// whose transcript is dropped by redaction (malformed JSONL) must be skipped
// instead of writing a metadata-only stub. The pre-redaction skip gate cannot
// catch this combination because both Transcript and FilesTouched are non-empty
// before the filter and redaction run.
func TestCondenseSession_SkipsWhenNoOverlapAndRedactionFails(t *testing.T) {
// whose transcript redaction fails (malformed JSONL) must NOT be skipped and
// must NOT be written as an empty stub. The redaction error now falls back to
// plain (regex-only) redaction, which never fails, so the checkpoint carries
// the fallback-redacted transcript instead of empty bytes. The pre-redaction
// skip gate cannot fire here because both Transcript and FilesTouched are
// non-empty before the filter and redaction run.
func TestCondenseSession_NoOverlapAndRedactionFails_FallsBackToPlainRedaction(t *testing.T) {
dir := setupGitRepo(t)
t.Chdir(dir)

Expand All @@ -140,7 +143,8 @@ func TestCondenseSession_SkipsWhenNoOverlapAndRedactionFails(t *testing.T) {
// skip gate won't fire.
sessionID := "redaction-failure-no-overlap"
transcriptPath := filepath.Join(dir, "fake-rollout.jsonl")
require.NoError(t, os.WriteFile(transcriptPath, []byte(`{"any":"jsonl line"}`+"\n"), 0o644))
transcript := []byte(`{"any":"jsonl line"}` + "\n")
require.NoError(t, os.WriteFile(transcriptPath, transcript, 0o644))

state := &SessionState{
SessionID: sessionID,
Expand All @@ -151,7 +155,7 @@ func TestCondenseSession_SkipsWhenNoOverlapAndRedactionFails(t *testing.T) {
FilesTouched: []string{"unrelated.txt"}, // not in committedFiles
}

// Force the redactor to fail so redactedTranscript ends up empty.
// Force the JSONL redactor to fail so the fallback path is exercised.
originalRedactor := redactSessionJSONLBytes
redactSessionJSONLBytes = func(_ context.Context, _ []byte) (redact.RedactedBytes, error) {
return redact.RedactedBytes{}, errors.New("simulated redaction failure")
Expand All @@ -164,7 +168,18 @@ func TestCondenseSession_SkipsWhenNoOverlapAndRedactionFails(t *testing.T) {

result, err := s.CondenseSession(context.Background(), repo, checkpointID, state, committedFiles)
require.NoError(t, err)
assert.True(t, result.Skipped, "should skip when no file overlap AND redaction drops transcript")
assert.False(t, result.Skipped, "redaction failure must not skip the session (plain fallback preserves the transcript)")

store := checkpoint.NewGitStore(repo, checkpoint.DefaultV1Refs())
committed, err := store.List(context.Background())
require.NoError(t, err)
require.NotEmpty(t, committed, "checkpoint should be written with the fallback-redacted transcript")

content, err := store.ReadLatestSessionContent(context.Background(), checkpointID)
require.NoError(t, err)
require.NotEmpty(t, content.Transcript, "transcript must not be empty bytes when JSONL redaction fails")
require.Equal(t, redact.AlreadyRedacted(redact.Bytes(transcript)).Bytes(), content.Transcript,
"transcript should be the plain-redaction fallback, not empty")
}

// filterFilesTouched should still apply the all-committed-files fallback for a
Expand Down
15 changes: 9 additions & 6 deletions cli/strategy/manual_commit_condensation.go
Original file line number Diff line number Diff line change
Expand Up @@ -466,26 +466,29 @@ func (s *ManualCommitStrategy) CondenseSession(ctx context.Context, repo *git.Re
}

// redactOrDrop runs redactSessionTranscript and, on failure, logs a warning
// and returns empty bytes. Drop-on-failure is the long-standing contract here:
// hooks have no retry path, and a failed redaction must not block the commit.
// and falls back to plain (regex-only) redaction. A redaction failure must
// never silently turn a snapshot into an empty blob — the JSONL pass only
// fails on unparseable content, which plain redaction still handles
// byte-for-byte.
func redactOrDrop(logCtx context.Context, transcript []byte, sessionID string, checkpointID id.CheckpointID) (redact.RedactedBytes, time.Duration) {
redactedTranscript, redactDuration, err := redactSessionTranscript(logCtx, transcript)
if err != nil {
logging.Warn(
logCtx, "failed to redact transcript secrets, dropping transcript for checkpoint",
logCtx, "failed to redact transcript secrets, falling back to plain redaction",
slog.String("session_id", sessionID),
slog.String("checkpoint_id", checkpointID.String()),
slog.String("error", err.Error()),
)
return redact.RedactedBytes{}, redactDuration
return redact.AlreadyRedacted(redact.Bytes(transcript)), redactDuration
}
return redactedTranscript, redactDuration
}

// skipIfPostRedactionEmpty returns a Skipped result when redaction emptied the
// transcript AND the filtered FilesTouched is also empty. Without this, a
// session that passed the pre-redaction gate but got its transcript dropped by
// a malformed-JSONL redaction error would write a metadata-only stub.
// session whose transcript was genuinely emptied by redaction (e.g. pure
// secret content) would write a metadata-only stub. Redaction errors no
// longer reach this gate — they fall back to plain redaction instead.
func skipIfPostRedactionEmpty(logCtx context.Context, redactedTranscript redact.RedactedBytes, sessionData *ExtractedSessionData, state *SessionState, checkpointID id.CheckpointID) *CondenseResult {
if redactedTranscript.Len() > 0 || len(sessionData.FilesTouched) > 0 {
return nil
Expand Down
8 changes: 6 additions & 2 deletions cli/strategy/manual_commit_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -2975,12 +2975,16 @@ func (s *ManualCommitStrategy) finalizeAllTurnCheckpoints(ctx context.Context, s
redactedTranscript, redactErr := redact.JSONLBytes(fullTranscript)
redactSpan.End()
if redactErr != nil {
// Fall back to the plain regex layers rather than storing an EMPTY
// transcript: a redaction failure must never silently turn a snapshot
// into an empty blob (the JSONL pass only fails on unparseable
// content, which plain redaction still handles byte-for-byte).
logging.Warn(
logCtx, "finalize: transcript redaction failed, dropping transcript",
logCtx, "finalize: JSONL transcript redaction failed, falling back to plain redaction",
slog.String("session_id", state.SessionID),
slog.String("error", redactErr.Error()),
)
redactedTranscript = redact.RedactedBytes{}
redactedTranscript = redact.AlreadyRedacted(redact.Bytes(fullTranscript))
}

// Post-commit emits regex-only blobs; the writer joins + redacts
Expand Down
9 changes: 6 additions & 3 deletions cli/strategy/manual_commit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4007,7 +4007,7 @@ func TestResolveFilesTouched_PrefersStateFallsBackToTranscript(t *testing.T) {
})
}

func TestCondenseSession_RedactionFailure_DropsTranscriptButWritesMetadata(t *testing.T) {
func TestCondenseSession_RedactionFailure_FallsBackToPlainRedaction(t *testing.T) {
originalRedact := redactSessionJSONLBytes
redactSessionJSONLBytes = func(context.Context, []byte) (redact.RedactedBytes, error) {
return redact.RedactedBytes{}, errors.New("forced redaction failure")
Expand Down Expand Up @@ -4078,8 +4078,11 @@ func TestCondenseSession_RedactionFailure_DropsTranscriptButWritesMetadata(t *te
}
require.True(t, found, "checkpoint metadata should be written even when transcript redaction fails")

_, err = store.ReadLatestSessionContent(context.Background(), checkpointID)
require.ErrorIs(t, err, checkpoint.ErrNoTranscript, "transcript should be dropped when redaction fails")
content, err := store.ReadLatestSessionContent(context.Background(), checkpointID)
require.NoError(t, err, "redaction failure should not abort condensation")
require.NotEmpty(t, content.Transcript, "transcript must not be empty bytes when redaction fails")
require.Equal(t, redact.AlreadyRedacted(redact.Bytes([]byte(transcript))).Bytes(), content.Transcript,
"transcript should be the plain-redaction fallback, not dropped")
}

func TestCommittedFilesExcludingMetadata(t *testing.T) {
Expand Down
Loading