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
12 changes: 6 additions & 6 deletions internal/agent/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,8 @@ func (s *Session) Replace(msgs []provider.Message) {
// pruning, or local metadata edits: a later autosave must use owned-rewrite
// conflict checks instead of mistaking the modified prefix for another writer.
//
// reason names the provider-visible change (e.g. "compact_auto", "snip",
// "rewind_truncate") and is queued for the next DrainContentRewriteReasons
// reason names the provider-visible change (e.g. "rewind_truncate",
// "guardian_merge") and is queued for the next DrainContentRewriteReasons
// call, which feeds cache-diagnostics attribution. Callers whose msgs only
// change local-only display metadata (never serialized to the provider) must
// use ReplaceLocalMetadata instead, so they don't misreport a cache-prefix
Expand Down Expand Up @@ -518,10 +518,10 @@ func (s *Session) RewriteVersion() int {
return s.rewriteVersion
}

// NeedsRewriteSave reports whether the history has been rewritten in memory
// (compaction, prune) since the last successful full save of this session.
// Snapshot paths use it to decide that the next write must be an owned
// rewrite instead of an append.
// NeedsRewriteSave reports whether the message log was rewritten in place —
// rather than appended to — since the last successful full save of this
// session. Snapshot paths use it to decide that the next write must be an
// owned rewrite instead of an append.
func (s *Session) NeedsRewriteSave() bool {
s.mu.RLock()
defer s.mu.RUnlock()
Expand Down
2 changes: 1 addition & 1 deletion internal/jobs/artifacts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestCompletedJobPersistsOutputAndReleasesMemory(t *testing.T) {

j.mu.Lock()
tailLen := len(j.tail)
result := j.result
result := j.outcome.text
artifactPath := j.artifactPath
j.mu.Unlock()

Expand Down
61 changes: 26 additions & 35 deletions internal/jobs/jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,19 +123,14 @@ type Job struct {
Label string
SessionID string

mu sync.Mutex
tail []byte
readOffset int64
status Status
result string
resultRead bool // result already surfaced by Output (task jobs stream nothing to buf)
startedAt int64
finishedAt int64
activityAt int64
runReturned bool
cancel context.CancelFunc
done chan struct{}
stalled bool
mu sync.Mutex
tail []byte
readOffset int64
status Status
clock jobClock
outcome jobOutcome
cancel context.CancelFunc
done chan struct{}

artifactPath string
artifactMetaPath string
Expand Down Expand Up @@ -326,7 +321,7 @@ type jobWriter struct{ j *Job }
func (w jobWriter) Write(p []byte) (int, error) {
w.j.mu.Lock()
defer w.j.mu.Unlock()
w.j.activityAt = nowMs()
w.j.clock.activityAt = nowMs()
w.j.tail = appendTail(w.j.tail, p, defaultTailBytes)
if w.j.artifactFile != nil {
if _, err := w.j.artifactFile.Write(p); err != nil {
Expand Down Expand Up @@ -391,10 +386,8 @@ func (m *Manager) startInvalid(parentSession, kind, label string, validationErr
Label: label,
SessionID: parentSession,
status: Failed,
startedAt: finishedAt,
activityAt: finishedAt,
finishedAt: finishedAt,
runReturned: true,
clock: jobClock{startedAt: finishedAt, activityAt: finishedAt, finishedAt: finishedAt},
outcome: jobOutcome{returned: true},
cancel: func() {},
done: make(chan struct{}),
artifactComplete: false,
Expand Down Expand Up @@ -479,8 +472,8 @@ func (m *Manager) writeJobMetaLocked(j *Job, st Status) error {
SessionID: j.SessionID,
OwnerID: m.ownerID,
Status: st,
StartedAt: j.startedAt,
FinishedAt: j.finishedAt,
StartedAt: j.clock.startedAt,
FinishedAt: j.clock.finishedAt,
ArtifactComplete: st != Running && j.artifactComplete && j.artifactErr == "",
ArtifactError: j.artifactErr,
LogPath: filepath.Base(j.artifactPath),
Expand Down Expand Up @@ -620,13 +613,13 @@ func (m *Manager) monitorStalled(parentSession string, j *Job) {
return
case <-timer.C:
j.mu.Lock()
if j.runReturned || j.status != Running {
if j.outcome.returned || j.status != Running {
j.mu.Unlock()
return
}
idle := time.Since(time.UnixMilli(j.activityAt))
if idle >= m.stalledWarning && !j.stalled {
j.stalled = true
idle := time.Since(time.UnixMilli(j.clock.activityAt))
if idle >= m.stalledWarning && !j.clock.stalled {
j.clock.stalled = true
j.mu.Unlock()
m.recordStalled(parentSession, j.ID, j.Kind, j.Label)
return
Expand Down Expand Up @@ -753,12 +746,12 @@ func (m *Manager) OutputForSession(parentSession, id string) (text string, statu
j.readOffset = int64(len(full))
}
}
// A task job streams nothing to the buffer — its answer lands in result. Once
// it is terminal with no buffered output, surface that result once so a task's
// answer is visible here too (bash_output's description promises task support).
if text == "" && j.status != Running && j.result != "" && !j.resultRead {
text = j.result
j.resultRead = true
// A task job streams nothing to the tail buffer — its answer lands in
// outcome.text. Surface it once when terminal with no buffered output, so a
// task's answer is visible here too (bash_output promises task support).
if text == "" && j.status != Running && j.outcome.text != "" && !j.outcome.read {
text = j.outcome.text
j.outcome.read = true
}
if j.artifactErr != "" {
if text != "" {
Expand Down Expand Up @@ -924,7 +917,7 @@ func (m *Manager) results(targets []*Job) []Result {
out := make([]Result, 0, len(targets))
for _, j := range targets {
j.mu.Lock()
text := j.result
text := j.outcome.text
if text == "" && j.artifactPath != "" {
text = j.readArtifactAllLocked()
}
Expand Down Expand Up @@ -972,7 +965,7 @@ func (m *Manager) RunningForSession(parentSession string) []View {
// runtime idle early. The public view remains "running" while a stop is
// in flight; clients may render a local "stopping" state after they
// request cancellation.
out = append(out, View{ID: j.ID, Kind: j.Kind, Label: j.Label, Status: string(Running), StartedAt: j.startedAt})
out = append(out, View{ID: j.ID, Kind: j.Kind, Label: j.Label, Status: string(Running), StartedAt: j.clock.startedAt})
j.mu.Unlock()
}
return out
Expand Down Expand Up @@ -1484,9 +1477,7 @@ func (m *Manager) loadSessionArtifacts(parentSession, sessionPath, dir string) {
Label: meta.Label,
SessionID: parentSession,
status: meta.Status,
startedAt: meta.StartedAt,
finishedAt: meta.FinishedAt,
activityAt: meta.FinishedAt,
clock: jobClock{startedAt: meta.StartedAt, finishedAt: meta.FinishedAt, activityAt: meta.FinishedAt},
done: done,
artifactPath: logPath,
artifactMetaPath: filepath.Join(dir, id+jobMetaExt),
Expand Down
23 changes: 23 additions & 0 deletions internal/jobs/jobstate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package jobs

// jobClock is a job's liveness timeline: unix-millisecond stamps advanced as the
// job runs, plus the latch the stalled warning sets once activityAt goes quiet.
// Grouping the latch with the stamp it derives from stops the two from
// describing different jobs, and keeps Job from listing every scalar flat.
// Guarded by Job.mu; this type takes no lock of its own.
type jobClock struct {
startedAt int64
finishedAt int64
activityAt int64
stalled bool
}

// jobOutcome is what the run function left behind: its text, whether the run
// returned at all, and whether Output already surfaced the text. All three are
// written once as the job terminates and read together afterwards. Guarded by
// Job.mu.
type jobOutcome struct {
text string
returned bool
read bool // text already surfaced by Output (task jobs stream nothing to the tail)
}
11 changes: 5 additions & 6 deletions internal/jobs/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ func (m *Manager) StartForSession(parentSession, kind, label string, run func(ct
Label: label,
SessionID: parentSession,
status: Running,
startedAt: startedAt,
activityAt: startedAt,
clock: jobClock{startedAt: startedAt, activityAt: startedAt},
cancel: cancel,
done: make(chan struct{}),
artifactPath: logPath,
Expand Down Expand Up @@ -78,7 +77,7 @@ func (m *Manager) runJob(ctx context.Context, j *Job, run func(context.Context,
defer m.wg.Done()
result, err := runRecovered(ctx, jobWriter{j}, run)
j.mu.Lock()
j.runReturned = true
j.outcome.returned = true
j.mu.Unlock()

var st Status
Expand All @@ -101,7 +100,7 @@ func (m *Manager) runJob(ctx context.Context, j *Job, run func(context.Context,
j.artifactErr = writeErr.Error()
}
} else {
j.result = result
j.outcome.text = result
}
j.tail = appendTail(j.tail, []byte(result), defaultTailBytes)
j.mu.Unlock()
Expand All @@ -117,7 +116,7 @@ func (m *Manager) runJob(ctx context.Context, j *Job, run func(context.Context,
if j.artifactErr != "" {
j.artifactComplete = false
}
j.finishedAt = finishedAt
j.clock.finishedAt = finishedAt
if targetDir != "" {
if moveErr := j.moveArtifactToDirLocked(targetDir); moveErr != nil {
j.noteArtifactErr("migration: " + moveErr.Error())
Expand All @@ -138,7 +137,7 @@ func (m *Manager) runJob(ctx context.Context, j *Job, run func(context.Context,
j.status = st
}
if j.artifactPath != "" && j.artifactComplete {
j.result = ""
j.outcome.text = ""
j.tail = nil
}
j.mu.Unlock()
Expand Down
5 changes: 2 additions & 3 deletions tools/repolint/baseline.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"layering": 1,
"marker": 0,
"narrative": 64,
"struct-state": 109,
"struct-state": 103,
"test-file-size": 68583
},
"files": {
Expand Down Expand Up @@ -1113,8 +1113,7 @@
"internal/jobs/jobs.go": {
"essay": 21,
"file-size": 1271,
"function-size": 12,
"struct-state": 6
"function-size": 12
},
"internal/jobs/jobs_extra_test.go": {
"essay": 1
Expand Down