Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions openviking/server/routers/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,34 +90,39 @@ async def list_tasks(
),
resource_id: Optional[str] = Query(None, description="Filter by resource ID (e.g. session_id)"),
limit: int = Query(50, le=200, description="Max results"),
offset: int = Query(0, ge=0, description="Number of results to skip"),
_ctx: RequestContext = Depends(get_request_context),
):
"""List background tasks with optional filters."""
tracker = get_task_tracker()
if _ctx.role == Role.ROOT:
fetch_limit = offset + limit
system_tasks = await tracker.list_tasks(
task_type=task_type,
status=status,
resource_id=resource_id,
limit=limit,
limit=fetch_limit,
Comment thread
axiomoth marked this conversation as resolved.
account_id=SYSTEM_TASK_ACCOUNT_ID,
user_id=SYSTEM_TASK_USER_ID,
)
cached_tasks = await tracker.list_tasks(
task_type=task_type,
status=status,
resource_id=resource_id,
limit=limit,
limit=fetch_limit,
)
tasks_by_id = {task.task_id: task for task in cached_tasks}
tasks_by_id.update({task.task_id: task for task in system_tasks})
tasks = sorted(tasks_by_id.values(), key=lambda task: task.created_at, reverse=True)[:limit]
tasks = sorted(tasks_by_id.values(), key=lambda task: task.created_at, reverse=True)[
offset : offset + limit
]
else:
tasks = await tracker.list_tasks(
task_type=task_type,
status=status,
resource_id=resource_id,
limit=limit,
offset=offset,
account_id=_ctx.account_id,
user_id=_ctx.user.user_id,
)
Expand Down
5 changes: 4 additions & 1 deletion openviking/service/task_tracker.py
Original file line number Diff line number Diff line change
Expand Up @@ -875,6 +875,7 @@ async def list_tasks(
status: Optional[str] = None,
resource_id: Optional[str] = None,
limit: int = 50,
offset: int = 0,
account_id: Optional[str] = None,
user_id: Optional[str] = None,
) -> List[TaskRecord]:
Expand All @@ -885,6 +886,7 @@ async def list_tasks(
status,
resource_id,
limit,
offset,
account_id,
user_id,
)
Expand All @@ -896,6 +898,7 @@ async def _list_tasks_on_owner(
status: Optional[str],
resource_id: Optional[str],
limit: int,
offset: int,
account_id: Optional[str],
user_id: Optional[str],
) -> List[TaskRecord]:
Expand All @@ -910,7 +913,7 @@ async def _list_tasks_on_owner(
if resource_id:
tasks = [t for t in tasks if t.resource_id == resource_id]
tasks.sort(key=lambda t: t.created_at, reverse=True)
return tasks[:limit]
return tasks[offset : offset + limit]

async def has_running(
self,
Expand Down
3 changes: 2 additions & 1 deletion tests/api_test/api/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -662,9 +662,10 @@ def list_tasks(
status: Optional[str] = None,
resource_id: Optional[str] = None,
limit: int = 50,
offset: int = 0,
) -> requests.Response:
endpoint = "/api/v1/tasks"
params = {"limit": limit}
params = {"limit": limit, "offset": offset}
if task_type:
params["task_type"] = task_type
if status:
Expand Down
67 changes: 66 additions & 1 deletion tests/server/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@
from openviking.server.identity import ResolvedIdentity, Role
from openviking.server.models import ERROR_CODE_TO_HTTP_STATUS, ErrorInfo, Response
from openviking.service.core import OpenVikingService
from openviking.service.task_store import PersistentTaskStore
from openviking.service.task_store import (
SYSTEM_TASK_ACCOUNT_ID,
SYSTEM_TASK_USER_ID,
PersistentTaskStore,
)
from openviking.service.task_tracker import (
TaskTracker,
get_task_tracker,
Expand Down Expand Up @@ -473,6 +477,67 @@ async def test_task_endpoints_are_user_scoped():
set_task_tracker(None)


async def test_root_task_list_applies_offset_after_merging_task_scopes():
set_task_tracker(None)
_set_fake_task_tracker()
tracker = get_task_tracker()
await tracker.create(
"session_commit",
resource_id="oldest",
account_id="acme",
user_id="alice",
)
middle = await tracker.create(
"session_commit",
resource_id="middle",
account_id=SYSTEM_TASK_ACCOUNT_ID,
user_id=SYSTEM_TASK_USER_ID,
)
await tracker.create(
"session_commit",
resource_id="latest",
account_id="acme",
user_id="alice",
)
app = _build_task_http_test_app(
ResolvedIdentity(role=Role.ROOT, account_id="root", user_id="root")
)
transport = httpx.ASGITransport(app=app)

async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client:
response = await client.get("/api/v1/tasks", params={"limit": 1, "offset": 1})

assert response.status_code == 200
assert [task["task_id"] for task in response.json()["result"]] == [middle.task_id]
set_task_tracker(None)


async def test_user_task_list_applies_offset_after_sorting():
set_task_tracker(None)
_set_fake_task_tracker()
tracker = get_task_tracker()
task_ids = []
for resource_id in ("oldest", "middle", "latest"):
task = await tracker.create(
"session_commit",
resource_id=resource_id,
account_id="acme",
user_id="alice",
)
task_ids.append(task.task_id)
app = _build_task_http_test_app(
ResolvedIdentity(role=Role.USER, account_id="acme", user_id="alice")
)
transport = httpx.ASGITransport(app=app)

async with httpx.AsyncClient(transport=transport, base_url="http://testserver") as client:
response = await client.get("/api/v1/tasks", params={"limit": 1, "offset": 1})

assert response.status_code == 200
assert [task["task_id"] for task in response.json()["result"]] == [task_ids[1]]
set_task_tracker(None)


# ---- Role-based access tests ----


Expand Down
9 changes: 9 additions & 0 deletions tests/test_task_tracker.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,15 @@ async def test_list_limit(tracker: TaskTracker):
assert len(tasks) == 3


async def test_list_offset_applies_after_sorting(tracker: TaskTracker):
for i in range(5):
await tracker.create("session_commit", resource_id=f"s{i}", **_owner_kwargs())

tasks = await tracker.list_tasks(limit=2, offset=1)

assert [task.resource_id for task in tasks] == ["s3", "s2"]


async def test_list_order_most_recent_first(tracker: TaskTracker):
await tracker.create("session_commit", resource_id="first", **_owner_kwargs())
await tracker.create("session_commit", resource_id="second", **_owner_kwargs())
Expand Down
6 changes: 6 additions & 0 deletions web-studio/src/gen/ov-client/types.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4347,6 +4347,12 @@ export type GetTasksData = {
* Max results
*/
limit?: number;
/**
* Offset
*
* Number of results to skip
*/
offset?: number;
};
url: '/api/v1/tasks';
};
Expand Down
109 changes: 109 additions & 0 deletions web-studio/src/routes/tasks/route.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'

import { fetchTasks } from './route'

const ovClientMocks = vi.hoisted(() => ({
getOvResult: vi.fn(),
getTasks: vi.fn(),
}))

vi.mock('#/lib/ov-client', () => ({
getOvResult: ovClientMocks.getOvResult,
getTasks: ovClientMocks.getTasks,
ovClient: {},
}))

beforeEach(() => {
ovClientMocks.getOvResult.mockReset()
ovClientMocks.getTasks.mockReset()
ovClientMocks.getOvResult.mockImplementation(async (request) => request)
})
Comment thread
axiomoth marked this conversation as resolved.

describe('fetchTasks', () => {
it('fetches the latest 300 tasks in API-sized pages', async () => {
const now = Math.floor(Date.now() / 1000)
ovClientMocks.getTasks
.mockResolvedValueOnce(
Array.from({ length: 200 }, (_, index) => ({
created_at: now,
task_id: `task-${index}`,
})),
)
.mockResolvedValueOnce(
Array.from({ length: 100 }, (_, index) => ({
created_at: now,
task_id: `task-${index + 200}`,
})),
)

const tasks = await fetchTasks('all', 'all', '24h')

expect(tasks).toHaveLength(300)
expect(ovClientMocks.getTasks).toHaveBeenNthCalledWith(1, {
query: {
limit: 200,
offset: 0,
status: undefined,
task_type: undefined,
},
})
expect(ovClientMocks.getTasks).toHaveBeenNthCalledWith(2, {
query: {
limit: 100,
offset: 200,
status: undefined,
task_type: undefined,
},
})
})

it('continues loading all task pages until the API returns a short page', async () => {
ovClientMocks.getTasks
.mockResolvedValueOnce(
Array.from({ length: 200 }, (_, index) => ({
task_id: `task-${index}`,
})),
)
.mockResolvedValueOnce([{ task_id: 'task-200' }])

const tasks = await fetchTasks('all', 'all', 'all')

expect(tasks).toHaveLength(201)
expect(ovClientMocks.getTasks).toHaveBeenNthCalledWith(2, {
query: {
limit: 200,
offset: 200,
status: undefined,
task_type: undefined,
},
})
expect(ovClientMocks.getTasks).toHaveBeenCalledTimes(2)
})

it('passes task type filters through every page', async () => {
ovClientMocks.getTasks.mockResolvedValue([])

await fetchTasks('session_commit', 'all', 'all')

expect(ovClientMocks.getTasks).toHaveBeenCalledWith({
query: {
limit: 200,
offset: 0,
status: undefined,
task_type: 'session_commit',
},
})
})

it('stops when an older API repeats the first page', async () => {
const page = Array.from({ length: 200 }, (_, index) => ({
task_id: `task-${index}`,
}))
ovClientMocks.getTasks.mockResolvedValue(page)

const tasks = await fetchTasks('all', 'all', 'all')

expect(tasks).toHaveLength(200)
expect(ovClientMocks.getTasks).toHaveBeenCalledTimes(2)
})
})
38 changes: 29 additions & 9 deletions web-studio/src/routes/tasks/route.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ type TaskTypeFilter =
| 'all'

const DEFAULT_PAGE_SIZE = 20
const TASK_API_PAGE_SIZE = 200
const MAX_TASKS = 300
const MAX_ALL_TASKS = 10000
const PAGE_SIZE_OPTIONS = [20, 50, 100] as const
const TASK_TYPE_OPTIONS: Exclude<TaskTypeFilter, 'all'>[] = [
'session_commit',
Expand Down Expand Up @@ -111,24 +113,42 @@ export function getEffectiveTaskStatus(taskItem: any, list: any[]): string {

export type TaskDataScope = '24h' | 'all'

async function fetchTasks(
export async function fetchTasks(
taskType: TaskTypeFilter,
status: TaskStatusFilter,
dataScope: TaskDataScope = '24h',
): Promise<TaskRecord[]> {
const maxTasks = dataScope === 'all' ? MAX_ALL_TASKS : MAX_TASKS
const query = {
limit: dataScope === 'all' ? 10000 : MAX_TASKS,
status: undefined,
task_type: taskType === 'all' ? undefined : taskType,
include_archived: dataScope === 'all' ? true : undefined,
}
try {
const result = await getOvResult<unknown>(
getTasks({
query: query as any,
}),
)
let fetched = normalizeTasks(result).sort(
const tasksById = new Map<string, TaskRecord>()
let offset = 0
while (offset < maxTasks) {
const limit = Math.min(TASK_API_PAGE_SIZE, maxTasks - offset)
const result = await getOvResult<unknown>(
getTasks({
query: {
...query,
limit,
offset,
},
}),
)
const page = normalizeTasks(result)
const previousSize = tasksById.size
for (const task of page) {
tasksById.set(task.task_id, task)
}
offset += page.length
if (page.length < limit) break
// Avoid repeating the same page forever when cached Studio assets talk
// to an older API that does not support the offset parameter yet.
if (tasksById.size === previousSize) break
}
let fetched = Array.from(tasksById.values()).sort(
(a, b) => Number(b.created_at || 0) - Number(a.created_at || 0),
)
if (dataScope === '24h') {
Expand Down