Skip to content

preserve media store across config reloads#2040

Open
Harshit-shrivastav wants to merge 1 commit intosipeed:mainfrom
Harshit-shrivastav:preserve-media-store-1
Open

preserve media store across config reloads#2040
Harshit-shrivastav wants to merge 1 commit intosipeed:mainfrom
Harshit-shrivastav:preserve-media-store-1

Conversation

@Harshit-shrivastav
Copy link

@Harshit-shrivastav Harshit-shrivastav commented Mar 26, 2026

Bug Fix

Prevents loss of uploaded file references when using /reload command.

Problem

When calling /reload, a new FileMediaStore was created with an empty refs map, causing all previously uploaded files to become inaccessible with error: media store: unknown ref: media://...

Solution

Reuse the existing MediaStore instance instead of recreating it on every reload. The media store is now only created once on initial startup.

Changes

  • pkg/gateway/gateway.go (line 468): Wrap media store creation in if runningServices.MediaStore == nil

Testing

  1. Start picoclaw with Telegram enabled
  2. Upload a file to the bot
  3. Run /reload command
  4. Ask bot to read the previously uploaded file
  5. File should now be accessible (previously would fail with "unknown ref" error)

Impact

  • Users can now safely use /reload without breaking file uploads
  • No breaking changes
  • Backward compatible

📝 Description

🗣️ Type of Change

  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 📖 Documentation update
  • ⚡ Code refactoring (no functional changes, no api changes)

🤖 AI Code Generation

  • 🤖 Fully AI-generated (100% AI, 0% Human)
  • 🛠️ Mostly AI-generated (AI draft, Human verified/modified)
  • 👨‍💻 Mostly Human-written (Human lead, AI assisted or none)

🔗 Related Issue

📚 Technical Context (Skip for Docs)

  • Reference URL:
  • Reasoning:

🧪 Test Environment

  • Hardware:
  • OS:
  • Model/Provider:
  • Channels:

📸 Evidence (Optional)

Click to view Logs/Screenshots

☑️ Checklist

  • My code/docs follow the style of this project.
  • I have performed a self-review of my own changes.
  • I have updated the documentation accordingly.

## Bug Fix

Prevents loss of uploaded file references when using `/reload` command.

## Problem

When calling `/reload`, a new `FileMediaStore` was created with an empty `refs` map, causing all previously uploaded files to become inaccessible with error:
media store: unknown ref: media://...

## Solution

Reuse the existing `MediaStore` instance instead of recreating it on every reload. The media store is now only created once on initial startup.

## Changes

- `pkg/gateway/gateway.go` (line 468): Wrap media store creation in `if runningServices.MediaStore == nil`

## Testing

1. Start picoclaw with Telegram enabled
2. Upload a file to the bot
3. Run `/reload` command
4. Ask bot to read the previously uploaded file
5. File should now be accessible (previously would fail with "unknown ref" error)

## Impact

- Users can now safely use `/reload` without breaking file uploads
- No breaking changes
- Backward compatible
Copilot AI review requested due to automatic review settings March 26, 2026 07:27
@CLAassistant
Copy link

CLAassistant commented Mar 26, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes /reload behavior in the gateway so previously uploaded media refs remain resolvable by avoiding recreation of the FileMediaStore during service restarts.

Changes:

  • Guard FileMediaStore creation in restartServices() so the existing store instance is reused across reloads.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +484 to +493
if runningServices.MediaStore == nil {
runningServices.MediaStore = media.NewFileMediaStoreWithCleanup(media.MediaCleanerConfig{
Enabled: cfg.Tools.MediaCleanup.Enabled,
MaxAge: time.Duration(cfg.Tools.MediaCleanup.MaxAge) * time.Minute,
Interval: time.Duration(cfg.Tools.MediaCleanup.Interval) * time.Minute,
})
if fms, ok := runningServices.MediaStore.(*media.FileMediaStore); ok {
fms.Start()
}
}
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stopAndCleanupServices(..., isReload=true) still calls fms.Stop() for the existing FileMediaStore, but this restart path only creates/starts the media store when runningServices.MediaStore == nil. Since FileMediaStore.Stop() permanently closes the stop channel and Start() is guarded by startOnce, cleanup cannot be restarted after a /reload, leaving media cleanup disabled (and new cleanup settings ignored). Consider not stopping the media store on reload, or recreating it while migrating existing refs / adding a reconfigurable cleaner.

Suggested change
if runningServices.MediaStore == nil {
runningServices.MediaStore = media.NewFileMediaStoreWithCleanup(media.MediaCleanerConfig{
Enabled: cfg.Tools.MediaCleanup.Enabled,
MaxAge: time.Duration(cfg.Tools.MediaCleanup.MaxAge) * time.Minute,
Interval: time.Duration(cfg.Tools.MediaCleanup.Interval) * time.Minute,
})
if fms, ok := runningServices.MediaStore.(*media.FileMediaStore); ok {
fms.Start()
}
}
runningServices.MediaStore = media.NewFileMediaStoreWithCleanup(media.MediaCleanerConfig{
Enabled: cfg.Tools.MediaCleanup.Enabled,
MaxAge: time.Duration(cfg.Tools.MediaCleanup.MaxAge) * time.Minute,
Interval: time.Duration(cfg.Tools.MediaCleanup.Interval) * time.Minute,
})
if fms, ok := runningServices.MediaStore.(*media.FileMediaStore); ok {
fms.Start()
}

Copilot uses AI. Check for mistakes.
Comment on lines +483 to +493

if runningServices.MediaStore == nil {
runningServices.MediaStore = media.NewFileMediaStoreWithCleanup(media.MediaCleanerConfig{
Enabled: cfg.Tools.MediaCleanup.Enabled,
MaxAge: time.Duration(cfg.Tools.MediaCleanup.MaxAge) * time.Minute,
Interval: time.Duration(cfg.Tools.MediaCleanup.Interval) * time.Minute,
})
if fms, ok := runningServices.MediaStore.(*media.FileMediaStore); ok {
fms.Start()
}
}
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block’s indentation/bracing doesn’t match Go formatting (spaces instead of tabs and the closing } alignment). Please run gofmt on this section to keep the file consistent and avoid noisy diffs.

Suggested change
if runningServices.MediaStore == nil {
runningServices.MediaStore = media.NewFileMediaStoreWithCleanup(media.MediaCleanerConfig{
Enabled: cfg.Tools.MediaCleanup.Enabled,
MaxAge: time.Duration(cfg.Tools.MediaCleanup.MaxAge) * time.Minute,
Interval: time.Duration(cfg.Tools.MediaCleanup.Interval) * time.Minute,
})
if fms, ok := runningServices.MediaStore.(*media.FileMediaStore); ok {
fms.Start()
}
}
if runningServices.MediaStore == nil {
runningServices.MediaStore = media.NewFileMediaStoreWithCleanup(media.MediaCleanerConfig{
Enabled: cfg.Tools.MediaCleanup.Enabled,
MaxAge: time.Duration(cfg.Tools.MediaCleanup.MaxAge) * time.Minute,
Interval: time.Duration(cfg.Tools.MediaCleanup.Interval) * time.Minute,
})
if fms, ok := runningServices.MediaStore.(*media.FileMediaStore); ok {
fms.Start()
}
}

Copilot uses AI. Check for mistakes.
@sipeed-bot sipeed-bot bot added the type: bug Something isn't working label Mar 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants