feat(web-admin): expand admin operations for v1.7.0#29
Conversation
- Add Web Admin action queue for bot-owned runtime operations - Add diagnostics controls for reloads, context cleanup, and health checks - Add awakening management, immediate summary/briefing, and tieba peek actions
- Remove sensitive filter path disclosure from Web Admin status - Keep sensitive_words.toml out of config editor responses - Refresh the sensitive filter on LLM config reload
- Document awakening, LLBot migration, and Web Admin operations - Align configuration reference, deployment notes, and command guide - Update changelog and roadmap for release preparation
There was a problem hiding this comment.
Bot Review
The PR adds substantial new functionality (action queue for cross-process bot operations, awakening management UI, diagnostics runtime controls) with generally solid implementation patterns. However, there are several correctness concerns: a TOCTOU race in the awakening config write path that can cause lost updates under concurrent requests, missing file locking on boredom group persistence, and an architectural issue where the web admin health endpoint instantiates a second LLM service singleton in the web admin process (opening duplicate SQLite connections to shared data). Additionally, the web_admin_actions module imports private internals from plugin modules, creating fragile coupling.
🔴 HIGH: TOCTOU race condition: reload_config (line 159) reads the file outside the FileLock, then _write_awakening_config (line 176) writes under lock. Between read and write, another concurrent PUT request can read, modify, and write a different change, and the second write will silently overwrite the first.
File: quickquip/app/web/routes/awakening.py
Move the reload_config call inside _write_awakening_config's locked section, or restructure _apply_group_settings to hold the lock for the entire read-modify-write sequence (reload → modify → write → reload).
🟡 MEDIUM: set_boredom_opt_in reads (_load_boredom_groups), modifies, then writes (_save_boredom_groups) with no file-level locking. While the temp-file+replace pattern in _save_boredom_groups provides atomic individual writes, concurrent requests can still lose updates because the read-modify-write sequence is not atomic.
File: quickquip/app/web/routes/awakening.py
Use FileLock (similar to _write_awakening_config) around the full read-modify-write cycle in set_boredom_opt_in, or store boredom groups in the same TOML config file that already has locking.
🟡 MEDIUM: get_health calls _ensure_llm_bindings() and get_llm_service() directly in the web admin process. Since the web admin is a separate process from the bot, this creates a second LLMService singleton in the web admin process, which opens its own SQLite connections to data/llm.db (shared with the bot via bind mount). Every web admin worker that hits this endpoint will create a new service instance, leading to duplicate config loading and potential lock contention on shared SQLite databases.
File: quickquip/app/web/routes/llm_runtime.py
Either: (a) make the health endpoint read-only and avoid creating a full LLM service (read config/DB files directly), or (b) route health checks through the action queue like other bot-owned operations, returning cached health status from a shared file updated periodically by the bot process.
🟡 MEDIUM: _execute_summary_now and _execute_briefing_now import private (underscore-prefixed) module-level internals from daily_summary_plugin.py and daily_briefing_plugin.py (e.g., _LOCAL_TZ, _mark_triggered, _on_cooldown, _run_generation, _send_long_message). These are internal implementation details of the plugins that can change without notice, creating fragile coupling.
File: quickquip/adapters/nonebot/web_admin_actions.py
Either promote the needed functions/constants to public API (remove underscore prefix and document them), or extract the shared logic into a dedicated module (e.g., quickquip/chat/daily_summary_executor.py) that both the plugin command handler and web_admin_actions can import.
🟡 MEDIUM: list_awakening exposes source_path (line 238) — the filesystem path to awakening.toml. While not as sensitive as sensitive_words.toml, the sensitive_filter endpoint intentionally removed config_path (diff line 122) for defense-in-depth. Exposing any server-side filesystem paths through the API is inconsistent with the project's security posture.
File: quickquip/app/web/routes/awakening.py
Remove source_path from the list_awakening response, or restrict it to an existence boolean plus a non-path identifier.
🟡 MEDIUM: get_health endpoint calls svc.format_health('0', chat_type='group', ...) with a hardcoded group_id of '0'. The health report's scope_key will always be '0', and settings lookup for group '0' will likely return defaults or fail. This produces a misleading health report that doesn't reflect any real group's state.
File: quickquip/app/web/routes/llm_runtime.py
Consider using a sentinel value like '__web_admin__' or computing health at the global config level rather than a per-group scope. Alternatively, accept an optional scope_key query parameter.
🟢 LOW: claim() manually constructs WebAdminAction objects (lines 124–136) instead of using _row_to_action, duplicating JSON deserialization logic. If _row_to_action is later updated, claim() will diverge.
File: quickquip/app/web/action_queue.py
Call _row_to_action(row) and then override status and updated_at on the returned object, or add a factory method that claim and list_recent both use.
🟢 LOW: _connect() does not set PRAGMA journal_mode=WAL for the action queue database, unlike AuditLogger which explicitly enables WAL. Without WAL, concurrent reads from list_recent (web admin process) will be blocked while claim/complete/fail (bot process) hold write locks, and vice versa.
File: quickquip/app/web/action_queue.py
Add conn.execute('PRAGMA journal_mode=WAL') in _connect() or _ensure_schema().
🟢 LOW: The formatActionTime function produces strings like '05-27 14:30:00' with no year. ISO timestamps from _utc_now() are correctly parsed by new Date(), but the month-day-only format is ambiguous for audit purposes when actions span year boundaries.
File: frontend/src/views/DiagnosticsView.vue
Include the year in the format, e.g., YYYY-MM-DD HH:mm:ss.
Automated review. Reply with @KHPilot[bot] to ask follow-up questions.
- Serialize awakening config read-modify-write and queue bot-side reloads - Route health checks through the bot action queue - Add WAL, stale action cleanup, and review coverage
Summary
Verification