Skip to content

feat: persist downloaded media for session lifetime (#99)#104

Merged
fitz123 merged 1 commit intomainfrom
issue-99-media-persistence
Apr 19, 2026
Merged

feat: persist downloaded media for session lifetime (#99)#104
fitz123 merged 1 commit intomainfrom
issue-99-media-persistence

Conversation

@fitz123
Copy link
Copy Markdown
Owner

@fitz123 fitz123 commented Apr 19, 2026

Summary

  • Photos/documents/animations downloaded for an agent session persist across turns (no missing-file error on follow-up)
  • Voice files keep current immediate-cleanup behavior (only transcript enters context)
  • New media-store.ts manages per-chat dirs under /tmp/bot-media/<chat> with global byte cap (default 200 MB, oldest-first eviction)
  • Files cleaned up on session close (idle timeout, explicit close, restart)
  • Per-message cleanup callbacks removed from photo/document handlers; error-path partial cleanup preserved
  • enforceMediaCap is best-effort (IO/permission errors logged, not thrown); media dirs created with mode 0o700

Rebased onto main after PR #101 merged. Supersedes closed PR #102.

Closes #99

Test plan

  • `cd bot && npm test` (media-store: 133/133 pass on rebased branch)
  • `cd bot && npx tsc --noEmit`
  • codex external review: NO ISSUES FOUND
  • claude review: no critical/major findings
  • copilot review: addressed

Generated with Claude Code

Copilot AI review requested due to automatic review settings April 19, 2026 12:14
Photos/documents/animations downloaded for an agent session persist
across turns (no missing-file error on follow-up). Voice files keep
current immediate-cleanup behavior (only transcript enters context).

- New media-store.ts manages per-chat dirs under /tmp/bot-media/<chat>
  with global byte cap (default 200 MB, oldest-first eviction)
- Files cleaned up on session close (idle timeout, explicit close, restart)
- Per-message cleanup callbacks removed from photo/document handlers
- enforceMediaCap is best-effort (IO/permission errors logged, not thrown)
- Media dirs created with mode 0o700
- Added example keyword to maxMediaBytes default comment to avoid
  gitleaks telegram-user-ids false positive on the 9-digit byte value

Closes #99

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@fitz123 fitz123 force-pushed the issue-99-media-persistence branch from 339d115 to 28b78ea Compare April 19, 2026 12:17
Copy link
Copy Markdown

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

This PR adds session-lifetime persistence for downloaded media (photos/documents/animations) so follow-up turns can reference previously downloaded files, while keeping voice media on immediate-cleanup semantics. It introduces a per-session media directory under /tmp/bot-media, adds a global byte cap with oldest-first eviction, and wires ownership/cleanup semantics through the message queue + stream relay so media is only discarded when a message never reaches an agent.

Changes:

  • Add media-store.ts to allocate per-session media paths, secure directories (0o700), enforce a global size cap, and clean up on session close/rotation.
  • Extend MessageQueue + relayStream with an onAgentOwnership signal to prevent reclaiming persistent media after the agent has accepted the prompt.
  • Add maxMediaBytes to session defaults/config validation and expand test coverage for media persistence and cleanup paths.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
docs/plans/completed/2026-04-18-issue-99-media-persistence.md Implementation plan and rationale for session-scoped media persistence + cap/cleanup semantics.
config.yaml Documents sessionDefaults.maxMediaBytes and eviction behavior.
bot/src/types.ts Adds maxMediaBytes to SessionDefaults.
bot/src/config.ts Validates maxMediaBytes and provides default (200MB).
bot/src/media-store.ts New module: secure per-session dirs, in-flight tracking, stale cleanup, and cap eviction.
bot/src/message-queue.ts Adds drop-only cleanup hooks and an ownership transfer signal to avoid discarding persistent media after prompt acceptance.
bot/src/stream-relay.ts Signals ownership on first stream event to coordinate persistent media lifecycle with the queue.
bot/src/telegram-bot.ts Switches media downloads to session media paths; removes per-turn deletion; integrates ownership/drop cleanup hooks.
bot/src/discord-bot.ts Updates queue→relay signature to pass ownership callback (no Discord media persistence added here).
bot/src/session-manager.ts Ensures media dir exists on spawn; cleans media on close/destroy; purges stale media on session discard/rotation.
bot/src/main.ts Notes why startup does not globally wipe media.
bot/src/tests/config-defaults.test.ts Tests default/validation behavior for maxMediaBytes.
bot/src/tests/hot-reload.test.ts Updates test config to include maxMediaBytes.
bot/src/tests/telegram-bot.test.ts Updates test config to include maxMediaBytes.
bot/src/tests/session-manager.test.ts Adds tests for media dir cleanup on destroy and stale purge behaviors.
bot/src/tests/message-queue.test.ts Adds coverage for drop-cleanup semantics and ownership transfer regressions.
bot/src/tests/media-store.test.ts Comprehensive tests for directory security, eviction, stale cleanup, and symlink protections.

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

Comment thread bot/src/media-store.ts
Comment on lines +213 to +217
export function enforceMediaCap(maxBytes: number): void {
// Best-effort housekeeping: never throw. An unrelated permission/IO error
// in another chat's dir must not fail the current download-enqueue path.
if (!existsSync(MEDIA_BASE)) return;

Copy link

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

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

enforceMediaCap scans and unlinks under MEDIA_BASE without first verifying that MEDIA_BASE is a real directory (not a symlink). If /tmp/bot-media is pre-squatted as a symlink, this eviction sweep could traverse the link target and delete files outside the intended tree. Consider reusing mediaBaseSafeToTouch() (or an equivalent lstatSync symlink check) at the start of enforceMediaCap and bailing out when unsafe; adding a regression test for this would help prevent reintroducing the issue.

Copilot uses AI. Check for mistakes.
@fitz123 fitz123 merged commit d2d5b07 into main Apr 19, 2026
1 check passed
@fitz123 fitz123 deleted the issue-99-media-persistence branch April 19, 2026 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Persist downloaded media beyond single turn so follow-ups can reference earlier files

2 participants