feat(media): integrate TTL cleanup into FileMediaStore

[ex-takashima]

Summary

• Integrate TTL-based background cleanup (L2 safety net) directly into FileMediaStore, so file deletion and in-memory ref removal happen atomically under the same mutex — no dangling references
• Add MediaCleanupConfig (enabled, max_age_minutes, interval_minutes) to tools.media_cleanup config section with sensible defaults (enabled, 30min max age, 5min interval)
• Wire up NewFileMediaStoreWithCleanup + Start()/Stop() in cmd_gateway.go

Design decisions

• L1/L2 hybrid: ReleaseAll(scope) remains the primary (L1) cleanup path for scope-based immediate deletion. CleanExpired() acts as L2 safety net for leaked entries
• Atomic cleanup: Both file removal and in-memory ref/scope cleanup happen under a single sync.RWMutex lock, preventing races between cleanup and resolve
• refToScope reverse map: Enables CleanExpired() to find and clean scope mappings without scanning all scopes
• nowFunc injection: Allows deterministic time control in tests without time.Sleep

Files changed

File	Change
pkg/media/store.go	storedAt, refToScope, MediaCleanerConfig, CleanExpired(), Start()/Stop()
pkg/media/store_test.go	8 new tests (expiry, mixed ages, empty scopes, lifecycle, concurrency, consistency)
pkg/config/config.go	MediaCleanupConfig struct + ToolsConfig.MediaCleanup field
pkg/config/defaults.go	Default: {Enabled: true, MaxAge: 30, Interval: 5}
cmd/picoclaw/cmd_gateway.go	Wiring: NewFileMediaStoreWithCleanup + Start()/Stop()

Test plan

• go test ./pkg/media/ -v — 15/15 PASS (7 existing + 8 new)
• go test ./pkg/config/ -v — all PASS
• go test ./pkg/media/ -race -v — needs cgo-enabled environment (CI)
• go build ./cmd/picoclaw/ — pre-existing cmd_onboard.go embed error on this branch

🤖 Generated with Claude Code

[Zhaoyikaiii]

Start function need match the Stop function also using s.once.Do to dispatch goroutine.

[Zhaoyikaiii]

there err path not close the goroutine?

[Zhaoyikaiii]

and should cutoff >= 0 ?

[ex-takashima]

Thanks for the review, @alexhoshina @Zhaoyikaiii! All feedback has been addressed:

Commit 9a16f3b (review fixes):

• CleanExpired: two-phase lock — collect under lock, delete files after unlock
• CleanExpired: log file removal errors
• CleanExpired: guard against zero MaxAge
• Start: protect with startOnce to prevent multiple goroutines
• cmd_gateway: call mediaStore.Stop() on error path

Commit 8e20644 (additional hardening):

• Start: guard against Interval <= 0 to prevent time.NewTicker panic
• ReleaseAll: also converted to two-phase lock pattern
• Added TestStartZeroIntervalNoPanic and TestStartZeroMaxAgeNoPanic

Commit ad736d7 (LINE channel improvement per @alexhoshina's request):

• Added StartTyping() using LINE's loading animation API (1:1 only)
• Integrated PlaceholderRecorder.RecordTypingStop in processEvent
• Relaxed callAPI status check to accept 2xx (loading API returns 202)

All tests pass (18/18 media, CI green). Ready for re-review.

[alexhoshina]

hi @ex-takashima, regarding the commit about line, it might be part of another PR. Our current PR should focus on mediastore.

[ex-takashima]

@alexhoshina Got it — LINE commit has been reverted from this PR and moved to a separate PR: #811. This PR now focuses solely on MediaStore TTL cleanup.

[alexhoshina]

LGTM

[nikolasdehor]

Clean implementation. The L1/L2 hybrid design, two-phase lock/unlock pattern, and nowFunc injection are all well-thought-out. Some feedback:

1. ReleaseAll and CleanExpired can race on the same entry.
Thread A calls ReleaseAll("scope1"), grabs the lock, collects paths, deletes from maps, releases the lock. Thread B calls CleanExpired, grabs the lock, finds the entry already gone (no problem). But consider: Thread A releases the lock and starts deleting files. Thread B (or another ReleaseAll) calls os.Remove on the same path. The second os.Remove gets ENOENT, which is handled by !os.IsNotExist(err). So this is safe. Good.

2. NewFileMediaStore() (without cleanup) initializes refToScope but not stop.
If someone calls Stop() on a store created with NewFileMediaStore(), the s.stop == nil check protects against a panic. Good. But Start() also checks s.stop == nil and returns early. This means Start() is a no-op on the basic constructor, which is the right behavior.

3. The log.Printf calls use stdlib log instead of the project logger.
The rest of picoclaw uses pkg/logger (e.g. logger.InfoCF, logger.WarnCF). Using log.Printf here is inconsistent and would bypass any log-level or structured-logging configuration. Consider using logger.WarnCF("media", ...) for consistency.

4. MaxAge and Interval are stored as int (minutes) in config but converted to time.Duration at the call site.
This is fine, but consider validating in cmd_gateway.go that MaxAge > 0 and Interval > 0 before constructing the config. Currently if someone sets max_age_minutes: 0 in config, CleanExpired() returns immediately (the guard on line if s.cleanerCfg.MaxAge <= 0), but Start() will also bail out early. The user gets no feedback that their config is effectively disabled.

5. Start() guard for zero interval prevents time.NewTicker panic -- good catch.
The check s.cleanerCfg.Interval <= 0 || s.cleanerCfg.MaxAge <= 0 prevents the panic. The dedicated test for this is appreciated.

6. Minor: the PR description mentions a race condition flag -race not being tested locally.
This is fine for the PR, but the concurrent test (TestConcurrentCleanupSafety) should ideally be run with -race in CI to catch any data races in the two-phase lock pattern.

Well-structured PR with good test coverage.