Skip to content

Commit 616ff01

Browse files
steveyeggeclaude
andcommitted
fix(channel): enforce RetentionHours in channel message retention
The RetentionHours field in ChannelFields was never enforced - only RetentionCount was checked. Now both EnforceChannelRetention and PruneAllChannels delete messages older than the configured hours. Also fixes sling tests that were missing TMUX_PANE and GT_TEST_NO_NUDGE guards, causing them to inject prompts into active tmux sessions during test runs. Fixes: gt-uvnfug Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 8d41f81 commit 616ff01

2 files changed

Lines changed: 76 additions & 24 deletions

File tree

internal/beads/beads_channel.go

Lines changed: 68 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ func (b *Beads) LookupChannelByName(name string) (*Issue, *ChannelFields, error)
382382

383383
// EnforceChannelRetention prunes old messages from a channel to enforce retention.
384384
// Called after posting a new message to the channel (on-write cleanup).
385-
// If channel has >= retainCount messages, deletes oldest until count < retainCount.
385+
// Enforces both count-based (RetentionCount) and time-based (RetentionHours) limits.
386386
func (b *Beads) EnforceChannelRetention(name string) error {
387387
// Get channel config
388388
_, fields, err := b.GetChannelBead(name)
@@ -393,8 +393,8 @@ func (b *Beads) EnforceChannelRetention(name string) error {
393393
return fmt.Errorf("channel not found: %s", name)
394394
}
395395

396-
// Skip if no retention limit
397-
if fields.RetentionCount <= 0 {
396+
// Skip if no retention limits configured
397+
if fields.RetentionCount <= 0 && fields.RetentionHours <= 0 {
398398
return nil
399399
}
400400

@@ -411,31 +411,51 @@ func (b *Beads) EnforceChannelRetention(name string) error {
411411
}
412412

413413
var messages []struct {
414-
ID string `json:"id"`
414+
ID string `json:"id"`
415+
CreatedAt string `json:"created_at"`
415416
}
416417
if err := json.Unmarshal(out, &messages); err != nil {
417418
return fmt.Errorf("parsing channel messages: %w", err)
418419
}
419420

420-
// Calculate how many to delete
421-
// We're being called after a new message is posted, so we want to end up with retainCount
422-
toDelete := len(messages) - fields.RetentionCount
423-
if toDelete <= 0 {
424-
return nil // No pruning needed
421+
// Track which messages to delete (use map to avoid duplicates)
422+
toDeleteIDs := make(map[string]bool)
423+
424+
// Time-based retention: delete messages older than RetentionHours
425+
if fields.RetentionHours > 0 {
426+
cutoff := time.Now().Add(-time.Duration(fields.RetentionHours) * time.Hour)
427+
for _, msg := range messages {
428+
createdAt, err := time.Parse(time.RFC3339, msg.CreatedAt)
429+
if err != nil {
430+
continue // Skip messages with unparseable timestamps
431+
}
432+
if createdAt.Before(cutoff) {
433+
toDeleteIDs[msg.ID] = true
434+
}
435+
}
425436
}
426437

427-
// Delete oldest messages (best-effort)
428-
for i := 0; i < toDelete && i < len(messages); i++ {
438+
// Count-based retention: delete oldest messages beyond RetentionCount
439+
if fields.RetentionCount > 0 {
440+
toDeleteByCount := len(messages) - fields.RetentionCount
441+
for i := 0; i < toDeleteByCount && i < len(messages); i++ {
442+
toDeleteIDs[messages[i].ID] = true
443+
}
444+
}
445+
446+
// Delete marked messages (best-effort)
447+
for id := range toDeleteIDs {
429448
// Use close instead of delete for audit trail
430-
_, _ = b.run("close", messages[i].ID, "--reason=channel retention pruning")
449+
_, _ = b.run("close", id, "--reason=channel retention pruning")
431450
}
432451

433452
return nil
434453
}
435454

436455
// PruneAllChannels enforces retention on all channels.
437456
// Called by Deacon patrol as a backup cleanup mechanism.
438-
// Uses a 10% buffer to avoid thrashing (only prunes if count > retainCount * 1.1).
457+
// Enforces both count-based (RetentionCount) and time-based (RetentionHours) limits.
458+
// Uses a 10% buffer for count-based pruning to avoid thrashing.
439459
func (b *Beads) PruneAllChannels() (int, error) {
440460
channels, err := b.ListChannelBeads()
441461
if err != nil {
@@ -444,38 +464,62 @@ func (b *Beads) PruneAllChannels() (int, error) {
444464

445465
pruned := 0
446466
for name, fields := range channels {
447-
if fields.RetentionCount <= 0 {
467+
// Skip if no retention limits configured
468+
if fields.RetentionCount <= 0 && fields.RetentionHours <= 0 {
448469
continue
449470
}
450471

451-
// Count messages
472+
// Get messages with timestamps
452473
out, err := b.run("list",
453474
"--type=message",
454475
"--label=channel:"+name,
455476
"--json",
456477
"--limit=0",
478+
"--sort=created",
457479
)
458480
if err != nil {
459481
continue // Skip on error
460482
}
461483

462484
var messages []struct {
463-
ID string `json:"id"`
485+
ID string `json:"id"`
486+
CreatedAt string `json:"created_at"`
464487
}
465488
if err := json.Unmarshal(out, &messages); err != nil {
466489
continue
467490
}
468491

469-
// 10% buffer - only prune if significantly over limit
470-
threshold := int(float64(fields.RetentionCount) * 1.1)
471-
if len(messages) <= threshold {
472-
continue
492+
// Track which messages to delete (use map to avoid duplicates)
493+
toDeleteIDs := make(map[string]bool)
494+
495+
// Time-based retention: delete messages older than RetentionHours
496+
if fields.RetentionHours > 0 {
497+
cutoff := time.Now().Add(-time.Duration(fields.RetentionHours) * time.Hour)
498+
for _, msg := range messages {
499+
createdAt, err := time.Parse(time.RFC3339, msg.CreatedAt)
500+
if err != nil {
501+
continue // Skip messages with unparseable timestamps
502+
}
503+
if createdAt.Before(cutoff) {
504+
toDeleteIDs[msg.ID] = true
505+
}
506+
}
507+
}
508+
509+
// Count-based retention with 10% buffer to avoid thrashing
510+
if fields.RetentionCount > 0 {
511+
threshold := int(float64(fields.RetentionCount) * 1.1)
512+
if len(messages) > threshold {
513+
toDeleteByCount := len(messages) - fields.RetentionCount
514+
for i := 0; i < toDeleteByCount && i < len(messages); i++ {
515+
toDeleteIDs[messages[i].ID] = true
516+
}
517+
}
473518
}
474519

475-
// Prune down to exactly retainCount
476-
toDelete := len(messages) - fields.RetentionCount
477-
for i := 0; i < toDelete && i < len(messages); i++ {
478-
if _, err := b.run("close", messages[i].ID, "--reason=patrol retention pruning"); err == nil {
520+
// Delete marked messages
521+
for id := range toDeleteIDs {
522+
if _, err := b.run("close", id, "--reason=patrol retention pruning"); err == nil {
479523
pruned++
480524
}
481525
}

internal/cmd/sling_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,7 @@ exit 0
616616
t.Setenv(EnvGTRole, "crew")
617617
t.Setenv("GT_CREW", "jv")
618618
t.Setenv("GT_POLECAT", "")
619+
t.Setenv("TMUX_PANE", "") // Prevent inheriting real tmux pane from test runner
619620

620621
cwd, err := os.Getwd()
621622
if err != nil {
@@ -637,6 +638,9 @@ exit 0
637638
slingDryRun = true
638639
slingNoConvoy = true
639640

641+
// Prevent real tmux nudge from firing during tests (causes agent self-interruption)
642+
t.Setenv("GT_TEST_NO_NUDGE", "1")
643+
640644
// EXPECTED: gt sling should use daemon mode and succeed
641645
// ACTUAL: verifyBeadExists uses --no-daemon and fails with sync error
642646
beadID := "jv-v599"
@@ -792,6 +796,7 @@ exit 0
792796
t.Setenv(EnvGTRole, "mayor")
793797
t.Setenv("GT_POLECAT", "")
794798
t.Setenv("GT_CREW", "")
799+
t.Setenv("TMUX_PANE", "") // Prevent inheriting real tmux pane from test runner
795800

796801
cwd, err := os.Getwd()
797802
if err != nil {
@@ -819,6 +824,9 @@ exit 0
819824
slingVars = nil
820825
slingOnTarget = "gt-abc123" // The bug bead we're applying formula to
821826

827+
// Prevent real tmux nudge from firing during tests (causes agent self-interruption)
828+
t.Setenv("GT_TEST_NO_NUDGE", "1")
829+
822830
if err := runSling(nil, []string{"mol-polecat-work"}); err != nil {
823831
t.Fatalf("runSling: %v", err)
824832
}

0 commit comments

Comments
 (0)