SEP-1847: Drop dead owner filter from task-history reads - #1376
Open
olucasandrade wants to merge 4 commits into
Open
SEP-1847: Drop dead owner filter from task-history reads#1376olucasandrade wants to merge 4 commits into
olucasandrade wants to merge 4 commits into
Conversation
Task.owner is an app namespace, not a user identity; the query parameter only produced 404s and never authorized anything. Pin team-wide read visibility in the dependency docstring and tests.
olucasandrade
added
the
qa passed
olucasandrade
marked this pull request as ready for review
August 20, 2026 15:58
olucasandrade
requested review from
a team,
marcuscruz-percona,
peter-o-addo and
yyyyyyyan
as code owners
August 20, 2026 15:58
Contributor
There was a problem hiding this comment.
Pull request overview
This focused PR removes the misleading task-owner filter while preserving authenticated, team-wide task-history access.
Changes:
- Removes
ownerfiltering fromget_task_history. - Updates OpenAPI types/specification and route coverage.
- Documents the user-facing API change.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
app/sep/deps.py |
Removes history owner filtering and clarifies visibility. |
tests/app/sep/test_deps.py |
Updates dependency-level coverage. |
tests/app/sep/routes/test_shared_route_auth.py |
Covers all five routes and their schema. |
frontend/packages/api/specs/sep.json |
Removes owner query parameters. |
frontend/packages/api/src/generated/sep.ts |
Regenerates TypeScript API types. |
changelog.d/SEP-1847.changed.md |
Records the API change. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
Drop the docstring wording assertion, build histories via the response factory, and reuse FastAPI status constants.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ownerquery parameter fromget_task_historyand the five task-history read routes (/files,/files/.../download,/stream-logs,/stream-logs/.../execution-events,/execution-events).Task.owneris an app namespace, not a user identity; the parameter only produced 404s.executed_by.get_task_by_name/make_task_dep()are unchanged.Tested
local-pg-backup-e2e, ownerBACKUP_PG,executed_bya user UUID, status SUCCESS). Task Manager or PostgreSQL backups → that run.GET /files/1310andGET /stream-logs/1310with?owner=ALTERSadded. Both still succeed (200 / SSE), not 404.GET http://localhost:8000/files/1310?owner=ALTERSis still 401./api/openapi.jsonlists noownerquery param on the five routes, while/api/apps/tasks/{task_name}still hasowner.Checklist
pyteston deps + shared-route-auth + files/logs/events +make_task_dep— 181 passed). Fullmake testwas 9843 passed / 82 failed / 429 skipped; failures are pre-existing (tests/sidecar/test_settings_env.pyand one xtrabackup AES round-trip) and unrelated to this changemake run-pre-commit)Database migrations generated if models changed ((N/A)make makemigrations)User-facing changes documented (README, inline help, UI text)(N/A — API/OpenAPI + changelog only)Configuration changes documented with examples(N/A)changelog.d/if the change is user-facing (make changelog-add), or confirmed N/A (internal-only change, or a same-release-cycle fix for an unreleased sibling ticket)