Sumnic/update aw webui task tracker#161
Conversation
Greptile SummaryThis PR adds a Task Tracker feature to aw-server — a new Flask blueprint with REST endpoints for managing tasks, time entries, app usages, and templates, backed by Peewee ORM models that share the existing SQLite connection. There are two P1 issues that should be resolved before merging:
Confidence Score: 3/5Not safe to merge — two P1 issues (startup crash on non-Peewee storage, CORS regression in production) need fixing first. The unconditional
|
| Filename | Overview |
|---|---|
| aw_server/server.py | Registers the task tracker blueprint unconditionally (breaks non-Peewee backends); removes the testing guard on the CORS dev-server origin, allowing it in production. |
| aw_server/task_tracker/routes.py | New REST API for task tracking; _get_aw_server_url() derives outbound HTTP target from the spoofable Host header (SSRF); select_task writes are non-atomic. |
| aw_server/task_tracker/models.py | Peewee ORM models for Task, TimeEntry, AppUsage, Template; imports private _db symbol from aw_datastore; updated_at is never refreshed on subsequent saves. |
| aw_server/task_tracker/init.py | Simple registration shim that calls init_tables() and registers the blueprint; correctness depends on Peewee storage being initialised first. |
| aw-webui | Submodule pointer bumped to a newer aw-webui commit; no server-side code concerns. |
Sequence Diagram
sequenceDiagram
participant Client
participant Flask as aw-server Flask
participant DB as SQLite Peewee
participant AW as ActivityWatch API
Client->>Flask: POST /api/0/task-tracker/tasks
Flask->>DB: Task.create(name, description)
DB-->>Flask: Task row
Flask-->>Client: 201 task
Client->>Flask: POST /api/0/task-tracker/tasks/id/select
Flask->>DB: Task.update all is_active=False
Flask->>DB: TimeEntry.update open entries end_time=now
Flask->>DB: task.save with is_active=True
Flask->>DB: TimeEntry.create for task
Flask-->>Client: 201 timeEntry
Client->>Flask: GET /api/0/task-tracker/activity-watch?taskId=X
Flask->>AW: GET /api/0/buckets/ via Host header URL
AW-->>Flask: buckets list
Flask->>AW: GET /api/0/buckets/bucket/events
AW-->>Flask: events list
Flask->>DB: AppUsage upsert per app
Flask-->>Client: appUsages list
Reviews (1): Last reviewed commit: "build(deps): update aw-webui" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # Used for development of aw-webui | ||
| cors_origins.append("http://127.0.0.1:27180/*") | ||
| # Allow Vue dev server (both testing and production/dev mode) | ||
| cors_origins.append("http://127.0.0.1:27180") |
There was a problem hiding this comment.
http://127.0.0.1:27180 (the Vue dev server) is now appended on every startup, including production deployments. Previously this was guarded behind if testing:. Any page served from that dev-server origin can now make authenticated cross-origin requests to a production ActivityWatch instance at will.
| cors_origins.append("http://127.0.0.1:27180") | |
| if testing: | |
| # Allow Vue dev server only during development | |
| cors_origins.append("http://127.0.0.1:27180") |
| # Register task tracker (creates tables + routes) | ||
| register_task_tracker(self) |
There was a problem hiding this comment.
register_task_tracker is called unconditionally, but models.py imports and uses _db from aw_datastore.storages.peewee. When storage_method is "memory" (the default and the one used in tests), Peewee storage is never initialised and _db is an uninitialised proxy. init_tables() will hit db_proxy.connect() on an unconfigured proxy and raise an OperationalError, crashing the entire server startup for every non-Peewee configuration.
Guard the registration so it only runs when the Peewee backend is in use, or make the task tracker initialise its own independent SQLite database.
| def _get_aw_server_url(): | ||
| """ | ||
| Determine the base URL of the running aw-server. | ||
| In testing mode this is http://127.0.0.1:5666, otherwise http://127.0.0.1:5600. | ||
| """ | ||
| # Try to infer from the current request's host | ||
| host = request.host.split(":")[0] | ||
| port = request.host.split(":")[1] if ":" in request.host else "5600" | ||
| return f"http://{host}:{port}" |
There was a problem hiding this comment.
_get_aw_server_url() builds the URL for all outbound requests to the ActivityWatch API from the incoming request's Host header. A client that spoofs Host: 192.168.1.10:5600 will make the server issue an HTTP request to an arbitrary internal host. For a localhost-only tool this is lower severity, but it remains a server-side request forgery vector.
Use a hard-coded or config-sourced base URL instead of deriving it from the request:
def _get_aw_server_url():
from flask import current_app
port = current_app.config.get("PORT", 5600)
return f"http://127.0.0.1:{port}"| created_at = pw.DateTimeField(default=datetime.now) | ||
| updated_at = pw.DateTimeField(default=datetime.now) |
There was a problem hiding this comment.
updated_at never refreshed on subsequent saves
updated_at uses default=datetime.now, which is evaluated once at object creation. Any subsequent task.save(), usage.save(), or template.save() call throughout routes.py leaves updated_at frozen at the original creation time. The same applies to TimeEntry (line 60) and Template (line 112).
Override save() on BaseModel to refresh updated_at automatically:
class BaseModel(pw.Model):
def save(self, *args, **kwargs):
if hasattr(self, "updated_at"):
self.updated_at = datetime.now()
return super().save(*args, **kwargs)|
|
||
| # Re-use the same peewee proxy that aw_datastore.storages.peewee uses. | ||
| # This way our models share the same SQLite database connection. | ||
| from aw_datastore.storages.peewee import _db as db_proxy |
There was a problem hiding this comment.
aw_datastore
_db is a private, implementation-internal variable (the underscore prefix signals this). Importing it from aw_datastore.storages.peewee tightly couples the task tracker to an internal detail that could change without warning in a future release of aw_datastore. Consider requesting that aw_datastore expose a stable public accessor, or maintain a separate SQLite connection owned by this sub-package.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| def select_task(task_id): | ||
| try: | ||
| task = Task.get_or_none(Task.id == task_id) | ||
| if not task: | ||
| return _error_response("Task not found", 404) | ||
|
|
||
| # Deactivate all tasks | ||
| Task.update(is_active=False).execute() | ||
|
|
||
| # Close any open time entries for other tasks (store in UTC) | ||
| now = datetime.now(timezone.utc) | ||
| TimeEntry.update(end_time=now).where(TimeEntry.end_time.is_null()).execute() | ||
|
|
||
| # Activate this task and create new time entry | ||
| task.is_active = True | ||
| task.save() | ||
|
|
||
| time_entry = TimeEntry.create(task=task, start_time=now) | ||
| return _json_response(time_entry.to_dict(), 201) |
There was a problem hiding this comment.
The select flow performs three separate DB writes without a transaction: Task.update(is_active=False), TimeEntry.update(end_time=now), and task.is_active = True; task.save(). A concurrent select call could interleave between these, leaving two tasks active at once or closing a newly-created time entry.
Wrap the body in with db_proxy.atomic(): to make the three writes a single database transaction.
1 participant
No description provided.