fix: improve Telegram message injection reliability

[brianjones-v4n]

Summary

• rawInject() now returns a boolean indicating success/failure and retries once before giving up (with a 300ms pause between attempts)
• Increased tmux send-keys timeout from 5s to 10s for text payloads, which fail on longer messages under load
• injectTelegramMessage() propagates the injection result so callers can detect failures
• Server route now checks the injection result — on failure, saves the message to /tmp/instar-telegram/failed/ and returns an error response instead of a false success
• cleanupStaleSessions() now uses a 1-hour TTL for completed job sessions (was 24h for all), and enforces a 50-session hard cap to prevent unbounded state file growth
• fileRoutes: fixed trailing-slash comparison in allowedPaths validation that caused false 403 errors depending on whether path.normalize() preserved trailing slashes

Problem

When tmux send-keys injection fails (timeout, session not ready, etc.), messages are silently dropped. The user sees ✓ Delivered but their message never reaches the Claude session. This is particularly problematic for longer messages and during periods of high tmux activity.

Test plan

• Send a short message via Telegram → verify injection succeeds and ✓ Delivered appears
• Send a long message (>500 chars) via Telegram → verify file-based injection works
• Kill a tmux session mid-injection → verify failure is detected, message saved to /tmp/instar-telegram/failed/, and error returned
• Verify cleanupStaleSessions() removes completed job sessions after 1 hour
• Verify file viewer works with paths that have trailing slashes in allowedPaths config

🤖 Generated with Claude Code

[JKHeadley]

Hi! I'm Echo, an AI developer agent that helps maintain this repo. I'll be reviewing your PR for code quality, security, and architectural fit. My reviews are advisory — @JKHeadley makes all final merge decisions.

A few things to know:

• I'm powered by Claude (Anthropic) and my reviews are generated, not hand-written
• If I request changes, I'll explain what needs fixing and why
• Feel free to ask questions — I'll respond to up to 2 reply rounds, then tag a human
• Trust levels are documented in CONTRIBUTING.md

Thanks for contributing!

[JKHeadley]

Echo's Review — PR #32

Recommendation: merge-with-changes

Summary

Solid reliability improvements to Telegram message injection. The core change — making rawInject return success/failure and adding a single retry — directly addresses the silent message drop problem. The session cleanup improvements and fileRoutes trailing-slash fix are welcome bonuses. Two concerns need addressing before merge.

Strengths

• Return type change (void → boolean) is the right API evolution — callers can now detect failures instead of assuming success
• Bounded retry (2 attempts, 300ms pause) is conservative and won't mask deeper issues
• Failed message preservation to /tmp prevents data loss — users won't lose messages silently
• DegradationReporter integration ensures injection failures surface in monitoring
• Session cleanup hard cap (50 completed sessions) prevents unbounded state growth
• fileRoutes fix for trailing-slash comparison is a genuine bug fix — path.normalize() behavior varies across Node versions
• Increased timeout (5s → 10s) for text payloads is sensible for longer messages

Concerns

• Sensitive messages persisted in /tmp without cleanup (moderate): Failed messages saved to /tmp/instar-telegram/failed/ contain user content. There's no TTL or cleanup mechanism — these files accumulate indefinitely. Add a cleanup step (e.g., delete files older than 24h during cleanupStaleSessions, or at minimum document that operators should monitor this directory).
• Type assertion (session as any).jobSlug (moderate): This bypasses TypeScript's type system. If the session type already has a jobSlug field, use proper typing. If not, add it to the session interface. as any creates a maintenance hazard — if the field name changes, this silently breaks.

Test Coverage

No automated tests added. The PR includes a manual test plan which is thorough, but the retry logic, failure-saving, and session cleanup changes all have testable branching logic that deserves unit tests. Particularly: retry succeeds on second attempt, failure saves to file, session hard cap prunes oldest first.

Security Findings

• /tmp/instar-telegram/failed/ directory stores message content. On multi-user systems, /tmp is world-readable by default. Consider using os.tmpdir() with restrictive permissions, or better yet, store in .instar/state/failed-messages/ where instar already manages access.

Verdict

The injection reliability fix is genuinely needed and well-implemented. The two concerns — unmanaged temp files with user content and the type assertion — should be addressed before merge. Neither is a heavy lift.

Next Steps

1. Add a cleanup mechanism for /tmp/instar-telegram/failed/ (or move to .instar/state/)
2. Replace (session as any).jobSlug with proper typing
3. Add tests for retry logic and failure-save behavior (see test plan items — they're already well-defined, just need to be automated)

---

Automated review by Echo — instar's developer agent. This review was generated by an AI system. For questions or concerns, please tag @JKHeadley.