Refactor agent initialization and fix permission flow#17
Conversation
Rikul
commented
Jul 1, 2026
- Fix ask_permission logic in agent.py (was commented out, now properly implemented)
- Update main.py to use message queue for initial prompt delivery
- Remove obsolete ask_permission tests that no longer apply
- Improve code structure by separating prompt handling from agent logic
- Fix ask_permission logic in agent.py (was commented out, now properly implemented) - Update main.py to use message queue for initial prompt delivery - Remove obsolete ask_permission tests that no longer apply - Improve code structure by separating prompt handling from agent logic
- Created agents.md documentation for architecture - Implemented command system with parser, registry, and dispatcher - Added /model command to show current AI model configuration - Integrated command system into agent (commands bypass LLM) - Made command system reusable across channels - Added /help, /config, /tools, /skills, /quit commands - Created demo_commands.md with usage examples - Command syntax supports arguments, flags, and quoted values
- Accept abstract Agent base class from main - Keep command system files from dev - Will adapt command system to new architecture
…ns to 250 + test updates - agent_loop: Compute on session_messages copy (self.messages[:] + user) → API calls isolated; persist user/assistant to messages/history *only* on success (if len(session_messages) > 2 → system+user+assistant) - Prevents partial history on empty/fail responses or tool loops - Bump max_iterations: Agent(100→250), BackgroundAgent(200→250), CliAgent(100→250) - Minor: log result[:250] (was 200); trim before loop; response_format conditional cleanups - tests/test_agent.py: + Non-empty system mock → triggers append condition + model_dump() mocks → consistent dicts for asserts + Patch MessageHistory.get_history → patch.object() + Assert agent.messages[-1].content; expect system in init (len==1) + Remove empty-messages test (obsolete post-system mock)
…o create pid file
…and len(session_message) > 2 to >= 2
…nentiate, factorial, is_prime, square_root
- app/scheduled_tasks.py: SQLite-backed ScheduledTasks class with tasks/task_outputs
tables, last_run field for interval tracking, and async run() loop
- app/tools/sched_tasks_tool.py: 6 tool wrappers (list, add, enable, disable, remove,
get_output) for agent-driven task management
- app/tool_calls.py: register scheduled task tools; add helper_tool_specs (excludes
scheduled task tools to prevent circular import via HelperAgent)
- app/helper_agent.py: lazy-import helper_tool_specs/run_tool inside agent_loop to
break the circular dependency
- app/bg_server.py: wire ScheduledTasks.run() into the main asyncio.gather
- app/sys_instructions.md: document the 6 new scheduled task tools with example
…easoning_content error
…k; fix: tool spec errors
- Replace EnableScheduledTask/DisableScheduledTask tools with UpdateScheduledTask
which supports partial updates to any task field (prompt, interval, repeat,
next_run, delivery_channel, enabled) via a single dynamic UPDATE query
- Make next_run required in add_task/AddScheduledTask (NOT NULL in schema)
- Fix get_skills_dir spec missing parameters field (caused 400 from providers)
- Switch helper_tool_specs from exclusion list to explicit allowlist
- Update sys_instructions.md with update_scheduled_task examples
- Enable PRAGMA foreign_keys = ON on all sqlite3 connections via _fk_conn helper; use BEGIN IMMEDIATE for the migration phase to prevent race conditions when two processes start simultaneously - Add idx_messages_conversation_id index (improves load_messages, list join, count_user_messages, fork queries) - MessageHistory._ensure_db() now adds conversation_id column if missing so it stays self-consistent without requiring ConversationStore to run first - get_default_sys_prompt() renders conversation_id and conversation_name in the system context block so the LLM is aware of the active conversation - Add ConversationAgent Protocol in commands.py for type-safe agent parameter - Fix load_conversation_cmd success message to report total message count (not user-only count) by reading from the list() result - Remove unused IncomingMessage import from background_agent.py - test_conversations: remove unused asyncio/runtime imports, replace time.sleep with deterministic timestamp pinning https://claude.ai/code/session_01AXnJ5ZXdQ9kxzsbjMtNfFH
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
MessageHistory already adds the conversation_id column; it should also own the index so queries that filter on conversation_id (load_messages, list join, fork) are covered even when ConversationStore hasn't run. https://claude.ai/code/session_01AXnJ5ZXdQ9kxzsbjMtNfFH
ORDER BY ASC LIMIT truncated to the oldest N messages, so after restart or /load-conversation on a long conversation the agent continued from stale early history. Changed to ORDER BY DESC LIMIT then reverse (same pattern as MessageHistory.get_history()), and added a regression test. https://claude.ai/code/session_01AXnJ5ZXdQ9kxzsbjMtNfFH
…sistent store.rename() sanitizes via _clean_name (trim, 80-char cap, empty fallback), but the command was propagating the raw user input into runtime and the response. Re-read the stored name after renaming so /status and the confirmation message always reflect the value actually saved to the DB. https://claude.ai/code/session_01AXnJ5ZXdQ9kxzsbjMtNfFH
- Remove channel-level CommandRegistry from Telegram and Discord.
Both channels now only handle /whoami inline; all other slash commands
are enqueued as plain IncomingMessages and dispatched by
BackgroundAgent.process_incoming() via its own registry.
- Add /model, /status, /stop to BackgroundAgent's registry.
Add _stop_cmd() to BackgroundAgent (was only on TelegramChannel before).
- Slugify auto-generated conversation names at the source (_slugify in
agent.py applied immediately after HelperAgent returns the title),
so the slug is stored in DB and propagated to runtime consistently.
- Clean up conversations.py: remove _clean_name() helper (inlined at
two call sites) and _slug() (logic moved to agent.py as _slugify).
- Use original next_run time (not current time) to compute the next schedule for repeating tasks. This prevents cumulative drift from task execution time. - Reduce poll interval from 60s to 30s for better responsiveness. - Add docstring explaining the catch-up behavior (Option A).
Changes _after_run to fast-forward past missed intervals rather than catching up one interval per poll cycle. This prevents a catch-up storm where many runs fire in rapid succession after prolonged downtime. If multiple intervals have passed since the original next_run, the next run is scheduled at: original_next_run + (intervals_passed + 1) * interval This keeps the original cadence locked while safely skipping over any missed executions.
* feat: add FastHTML web channel with clean chat UI Adds a WebChannel class (app/channels/web_channel.py) that serves both a FastHTML chat UI at GET / and a JSON WebSocket endpoint at WS /ws on the same uvicorn server. The UI features dark theme, user/AI message bubbles, auto-reconnect with status indicator, and a streaming-friendly input area. bg_server.py wires the new channel when [websocket] config is present; config.py adds WEBSOCKET_HOST/PORT env var overrides.
…dded /trace on|off command for bg agents. Tests for the trace feature.
…oding) (#13) * Use files metadata for all attachments * Attachments: 5MB combined limit, placeholder in history after first turn - session_messages for the current turn still carries full base64 data so the LLM sees the files; subsequent turns store a plain-text placeholder instead: "message [Attachment: ~/file.pdf]" - self.messages always holds plain strings now, eliminating token bloat across turns, DB storage concerns, and any need for redaction helpers - Add tests for the empty-string path, the missing-file-bypasses-size-check case, and the no-history-write-on-fail Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
* Add multi-file attachment support to the web channel Add a paperclip button next to the chat input that opens a native multi-file picker. Selected files render as removable chips and are uploaded via a new POST /api/upload endpoint, which stores them under $ANOTHERBOT_HOME/uploads with a UUID prefix and returns their basenames. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…laceholder shows filename only
…ory, conversations (#16) * fix: close leaked sqlite connections in scheduled_tasks, message_history, conversations with sqlite3.connect(...) as conn only commits/rolls back the transaction on exit, it never closes the connection or releases its file descriptor. ScheduledTasks.run() opens fresh connections every 30s forever, so over enough uptime these leak until the process hits its fd ulimit, causing "unable to open database file" everywhere (scheduled tasks, /list, and likely the Telegram poller's sockets too). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UfuBc1EqfevpENRBAmTMrA * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
… errors Scheduled tasks, message history, and conversations all write to the same sqlite file from concurrent asyncio tasks/channels. The default rollback journal mode plus a short busy timeout meant contention (e.g. a scheduled task writing task_outputs while the web channel writes messages) eventually exceeded the timeout and raised "database is locked". Centralize connection creation in config.get_db_connection(), which turns on WAL (so readers don't block the writer) and raises busy_timeout to 30s.
There was a problem hiding this comment.
Code Review
This pull request introduces file attachment and upload capabilities to the WebChannel, adds an LLM call tracing mechanism with a corresponding /trace command, and refactors database access to use a shared connection helper with WAL mode enabled. Feedback on the changes highlights a potential connection leak risk across several database operations in message_history.py and scheduled_tasks.py where exceptions could bypass conn.close(); wrapping these operations in try...finally blocks is recommended to ensure connections are reliably closed.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| with get_db_connection(self.db_path) as conn: | ||
| conn.execute( | ||
| "INSERT INTO messages (channel, role, content, timestamp, est_tokens, conversation_id) " | ||
| "VALUES (?, ?, ?, ?, ?, ?)", | ||
| (self.channel, role, content, timestamp, est, conversation_id), | ||
| ) | ||
| conn.commit() | ||
| conn.close() |
There was a problem hiding this comment.
The database connection conn is opened inside a with block, but sqlite3.Connection context managers do not automatically close the connection on exit (they only commit or rollback transactions). If an exception occurs inside the with block, conn.close() is bypassed, leading to a connection leak. Use a try...finally block to ensure the connection is always closed. Additionally, calling conn.commit() explicitly inside the with conn: block is redundant as the context manager handles transaction commit on successful exit.
| with get_db_connection(self.db_path) as conn: | |
| conn.execute( | |
| "INSERT INTO messages (channel, role, content, timestamp, est_tokens, conversation_id) " | |
| "VALUES (?, ?, ?, ?, ?, ?)", | |
| (self.channel, role, content, timestamp, est, conversation_id), | |
| ) | |
| conn.commit() | |
| conn.close() | |
| conn = get_db_connection(self.db_path) | |
| try: | |
| with conn: | |
| conn.execute( | |
| "INSERT INTO messages (channel, role, content, timestamp, est_tokens, conversation_id) " | |
| "VALUES (?, ?, ?, ?, ?, ?)", | |
| (self.channel, role, content, timestamp, est, conversation_id), | |
| ) | |
| finally: | |
| conn.close() |
| with get_db_connection(self.db_path) as conn: | ||
| rows = conn.execute("""SELECT role, content FROM messages | ||
| WHERE channel = ? | ||
| ORDER BY id DESC LIMIT ?""", (self.channel, limit)).fetchall() | ||
| return [{"role": row[0], "content": row[1]} for row in reversed(rows)] | ||
| conn.close() |
There was a problem hiding this comment.
The database connection conn is opened inside a with block, but if an exception occurs during query execution, conn.close() is bypassed, leading to a connection leak. Use a try...finally block to ensure the connection is always closed. Since this is a read-only query, a transaction context manager (with conn:) is not required.
| with get_db_connection(self.db_path) as conn: | |
| rows = conn.execute("""SELECT role, content FROM messages | |
| WHERE channel = ? | |
| ORDER BY id DESC LIMIT ?""", (self.channel, limit)).fetchall() | |
| return [{"role": row[0], "content": row[1]} for row in reversed(rows)] | |
| conn.close() | |
| conn = get_db_connection(self.db_path) | |
| try: | |
| rows = conn.execute("""SELECT role, content FROM messages | |
| WHERE channel = ? | |
| ORDER BY id DESC LIMIT ?""", (self.channel, limit)).fetchall() | |
| finally: | |
| conn.close() |
| with get_db_connection() as conn: | ||
| conn.execute("DELETE FROM tasks WHERE name = ?", (name,)) | ||
| conn.commit() | ||
| conn.close() |
There was a problem hiding this comment.
The database connection conn is opened inside a with block, but sqlite3.Connection context managers do not automatically close the connection on exit (they only commit or rollback transactions). If an exception occurs during task deletion, conn.close() is bypassed, leading to a connection leak. Use a try...finally block to ensure the connection is always closed.
| with get_db_connection() as conn: | |
| conn.execute("DELETE FROM tasks WHERE name = ?", (name,)) | |
| conn.commit() | |
| conn.close() | |
| conn = get_db_connection() | |
| try: | |
| with conn: | |
| conn.execute("DELETE FROM tasks WHERE name = ?", (name,)) | |
| finally: | |
| conn.close() |
| with get_db_connection() as conn: | ||
| conn.execute(""" | ||
| INSERT INTO task_outputs (name, prompt, output, status, duration_secs, timestamp) | ||
| VALUES (?, ?, ?, ?, ?, ?) | ||
| """, (name, prompt, output, status, duration_secs, datetime.now().isoformat())) | ||
| conn.commit() | ||
| conn.close() |
There was a problem hiding this comment.
The database connection conn is opened inside a with block, but sqlite3.Connection context managers do not automatically close the connection on exit (they only commit or rollback transactions). If an exception occurs during output insertion, conn.close() is bypassed, leading to a connection leak. Use a try...finally block to ensure the connection is always closed.
| with get_db_connection() as conn: | |
| conn.execute(""" | |
| INSERT INTO task_outputs (name, prompt, output, status, duration_secs, timestamp) | |
| VALUES (?, ?, ?, ?, ?, ?) | |
| """, (name, prompt, output, status, duration_secs, datetime.now().isoformat())) | |
| conn.commit() | |
| conn.close() | |
| conn = get_db_connection() | |
| try: | |
| with conn: | |
| conn.execute(""" | |
| INSERT INTO task_outputs (name, prompt, output, status, duration_secs, timestamp) | |
| VALUES (?, ?, ?, ?, ?, ?) | |
| """, (name, prompt, output, status, duration_secs, datetime.now().isoformat())) | |
| finally: | |
| conn.close() |
| with get_db_connection() as conn: | ||
| rows = conn.execute(""" | ||
| SELECT prompt, output, status, duration_secs, timestamp FROM task_outputs | ||
| WHERE name = ? | ||
| ORDER BY id DESC LIMIT ? | ||
| """, (name, num_entries)).fetchall() | ||
| conn.close() |
There was a problem hiding this comment.
The database connection conn is opened inside a with block, but if an exception occurs during query execution, conn.close() is bypassed, leading to a connection leak. Use a try...finally block to ensure the connection is always closed. Since this is a read-only query, a transaction context manager (with conn:) is not required.
| with get_db_connection() as conn: | |
| rows = conn.execute(""" | |
| SELECT prompt, output, status, duration_secs, timestamp FROM task_outputs | |
| WHERE name = ? | |
| ORDER BY id DESC LIMIT ? | |
| """, (name, num_entries)).fetchall() | |
| conn.close() | |
| conn = get_db_connection() | |
| try: | |
| rows = conn.execute(""" | |
| SELECT prompt, output, status, duration_secs, timestamp FROM task_outputs | |
| WHERE name = ? | |
| ORDER BY id DESC LIMIT ? | |
| """, (name, num_entries)).fetchall() | |
| finally: | |
| conn.close() |
| with get_db_connection() as conn: | ||
| conn.execute("""UPDATE tasks SET last_run = ?, next_run = ?, run_count = run_count + 1 | ||
| WHERE name = ?""", (now.isoformat(), next_run, name)) | ||
| conn.commit() | ||
| conn.close() |
There was a problem hiding this comment.
The database connection conn is opened inside a with block, but sqlite3.Connection context managers do not automatically close the connection on exit (they only commit or rollback transactions). If an exception occurs during task update, conn.close() is bypassed, leading to a connection leak. Use a try...finally block to ensure the connection is always closed.
| with get_db_connection() as conn: | |
| conn.execute("""UPDATE tasks SET last_run = ?, next_run = ?, run_count = run_count + 1 | |
| WHERE name = ?""", (now.isoformat(), next_run, name)) | |
| conn.commit() | |
| conn.close() | |
| conn = get_db_connection() | |
| try: | |
| with conn: | |
| conn.execute("""UPDATE tasks SET last_run = ?, next_run = ?, run_count = run_count + 1 | |
| WHERE name = ?""", (now.isoformat(), next_run, name)) | |
| finally: | |
| conn.close() |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a620045301
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ts = datetime.now().strftime("%m%d%Y_%H%M%S") | ||
| path = tracedir / f"trace_{ts}.json" |
There was a problem hiding this comment.
Make trace filenames collision-proof
When two traced LLM calls complete within the same second, which is easy with concurrent web clients or rapid CLI prompts, both calls write to the same trace_MMDDYYYY_HHMMSS.json path and the later one silently overwrites the earlier trace. Include microseconds or a UUID in the filename so tracing does not lose calls under normal concurrent usage.
Useful? React with 👍 / 👎.