feat: complete authored pull workspace access (Closes #1348)
All checks were successful
CI / lint (pull_request) Successful in 3m15s
CI / build-release (pull_request) Successful in 6s
CI / browser-journey (pull_request) Successful in 5m22s
CI / release-candidate (pull_request) Has been skipped

This commit is contained in:
timmy 2026-08-24 12:10:49 +00:00
parent 050390d6f6
commit 2102b0e5da
7 changed files with 264 additions and 44 deletions

View File

@ -187,17 +187,27 @@ function ownershipExitMessage(item, action, transitionResult) {
'. Next work item opened.' : '. Choose the next ready Today item.');
}
function resetOwnershipControls(doc, item, checkpointed) {
function applyOwnershipCapabilities(doc, capabilities = null) {
const ownership = doc.querySelector('#pull-ownership');
const assigned = capabilities?.assigned !== false;
ownership.hidden = !assigned;
ownership.inert = !assigned;
return assigned;
}
function resetOwnershipControls(doc, item, checkpointed, capabilities = null) {
const qs = selector => doc.querySelector(selector);
const assigned = applyOwnershipCapabilities(doc, capabilities);
qs('#pull-ownership').open = false;
qs('#pull-handoff-recipient').innerHTML = '<option value="">Select a teammate</option>';
qs('#pull-handoff-recipient').disabled = true;
qs('#confirm-pull-handoff').disabled = true;
qs('#confirm-pull-handoff').textContent = checkpointed(item) ? 'Hand off & next' : 'Confirm handoff';
qs('#load-pull-handoff').disabled = false;
qs('#release-pull').disabled = false;
qs('#load-pull-handoff').disabled = !assigned;
qs('#release-pull').disabled = !assigned;
qs('#release-pull').textContent = checkpointed(item) ? 'Release & next' : 'Release assignment';
qs('#pull-handoff-status').textContent = 'Load teammates to transfer ownership.';
qs('#pull-handoff-status').textContent = assigned ?
'Load teammates to transfer ownership.' : 'Assignment controls are not available for this pull request.';
qs('#edit-pull-content').hidden = true;
qs('#pull-edit-form').hidden = true;
qs('#pull-edit-status').textContent = '';
@ -227,6 +237,7 @@ function bindReviewRequestControls(doc, controller, getSelected, getDetail, getL
load.dataset.reviewRequestBound = 'true';
controller.setReviewDetail = detail => {
resetReviewRequestControls(doc, detail);
applyOwnershipCapabilities(doc, detail?.capabilities);
controller.edit?.setDetail(detail, Boolean(detail?.saved_at));
const publishable = detail?.state === 'open' && detail?.draft === true &&
detail?.author === getLogin?.() && detail?.head_sha;

View File

@ -1984,6 +1984,14 @@ async def pull_review_candidates(repository: str, number: int) -> list[dict]:
][:25]
def _login_can_manage_pull(login: str, pull: dict) -> bool:
author = pull.get("user") if isinstance(pull.get("user"), dict) else {}
return (
author.get("login", "").casefold() == login.casefold()
or _login_in_users(login, pull.get("assignees"))
)
async def request_assigned_pull_review(
repository: str, number: int, reviewer: str, expected_head_sha: str
) -> dict:
@ -1994,7 +2002,7 @@ async def request_assigned_pull_review(
if (
pull.get("state") != "open"
or pull.get("merged") is True
or not _login_in_users(login, pull.get("assignees"))
or not _login_can_manage_pull(login, pull)
or head.get("sha") != expected_head_sha
):
raise IssueNotAvailableError("Pull request is no longer eligible for review request")
@ -2050,7 +2058,6 @@ async def publish_authored_assigned_pull(
or pull.get("merged") is True
or pull.get("draft") is not True
or author.get("login", "").casefold() != login.casefold()
or not _login_in_users(login, pull.get("assignees"))
or head.get("sha") != expected_head_sha
or not ready_title
or ready_title == title
@ -2106,7 +2113,7 @@ async def cancel_assigned_pull_review(
if (
pull.get("state") != "open"
or pull.get("merged") is True
or not _login_in_users(login, pull.get("assignees"))
or not _login_can_manage_pull(login, pull)
or head.get("sha") != expected_head_sha
or reviewer not in requested
):
@ -2711,7 +2718,7 @@ async def resolve_work_route(
and (
(kind == "issue" and not isinstance(target.get("pull_request"), dict) and assigned)
or (kind == "filed" and not isinstance(target.get("pull_request"), dict) and authored)
or (kind == "pull" and assigned)
or (kind == "pull" and (assigned or authored))
or (kind == "review" and requested)
)
)
@ -2730,6 +2737,14 @@ async def resolve_work_route(
"is_assigned": assigned,
"work_reasons": ["created_by_me"],
} if kind == "filed" else {}),
**({
"is_assigned": assigned,
"work_reasons": [
reason for reason, present in (
("assigned_to_me", assigned), ("authored_by_me", authored)
) if present
],
} if kind == "pull" else {}),
}
@ -2809,6 +2824,18 @@ async def is_requested_review(repository: str, number: int) -> bool:
)
async def pull_workspace_capabilities(repository: str, number: int) -> dict[str, bool]:
login, pull = await _current_login_and_target(
f"repos/{repository}/pulls/{number}"
)
author = pull.get("user") if isinstance(pull.get("user"), dict) else {}
open_pull = pull.get("state") == "open" and pull.get("merged") is not True
return {
"authored": open_pull and author.get("login", "").casefold() == login.casefold(),
"assigned": open_pull and _login_in_users(login, pull.get("assignees")),
}
async def is_assigned_pull(repository: str, number: int) -> bool:
login, pull = await _current_login_and_target(
f"repos/{repository}/pulls/{number}"
@ -2833,8 +2860,7 @@ async def update_authored_assigned_pull(
if (
pull.get("state") != "open"
or pull.get("merged") is True
or author.get("login") != login
or not _login_in_users(login, pull.get("assignees"))
or author.get("login", "").casefold() != login.casefold()
):
raise IssueNotAvailableError("pull request not found")
if head.get("sha") != expected_head_sha:

View File

@ -6173,8 +6173,10 @@ async def edit_assigned_pull_comment(
comment_id: int = PathParam(gt=0),
):
repository = f"{owner}/{repo}"
if not await gitea_proxy.is_assigned_pull(repository, number):
raise HTTPException(status_code=404, detail="Assigned pull request not found")
if not _has_pull_workspace_access(
await _pull_workspace_capabilities(repository, number)
):
raise HTTPException(status_code=404, detail="Pull request not found")
return await _edit_conversation_comment(repository, number, comment_id, comment.body)
@ -6190,8 +6192,10 @@ async def delete_assigned_pull_comment(
),
):
repository = f"{owner}/{repo}"
if not await gitea_proxy.is_assigned_pull(repository, number):
raise HTTPException(status_code=404, detail="Assigned pull request not found")
if not _has_pull_workspace_access(
await _pull_workspace_capabilities(repository, number)
):
raise HTTPException(status_code=404, detail="Pull request not found")
return await _delete_conversation_comment(
request, step_up_grant, repository, number, comment_id
)
@ -6555,14 +6559,28 @@ async def requested_review_checks(
)
async def _pull_workspace_capabilities(repository: str, number: int) -> dict[str, bool]:
# Preserve the established fast path for assigned work while widening access
# only after Gitea confirms authorship of the same open pull request.
if await gitea_proxy.is_assigned_pull(repository, number):
return {"authored": False, "assigned": True}
return await gitea_proxy.pull_workspace_capabilities(repository, number)
def _has_pull_workspace_access(capabilities: dict[str, bool]) -> bool:
return capabilities.get("authored") is True or capabilities.get("assigned") is True
@app.get("/api/v1/repos/{owner}/{repo}/pulls/{number}/detail")
async def assigned_pull_detail(owner: str, repo: str, number: int = PathParam(gt=0)):
repository = f"{owner}/{repo}"
async def load_assigned_pull():
if not await gitea_proxy.is_assigned_pull(repository, number):
raise HTTPException(status_code=404, detail="Assigned pull request not found")
return await gitea_proxy.pull_completion_detail(repository, number)
capabilities = await _pull_workspace_capabilities(repository, number)
if not _has_pull_workspace_access(capabilities):
raise HTTPException(status_code=404, detail="Pull request not found")
detail = await gitea_proxy.pull_completion_detail(repository, number)
return {**detail, "capabilities": capabilities}
try:
return await asyncio.wait_for(
@ -6655,9 +6673,11 @@ async def assigned_pull_review_data(
repository = f"{owner}/{repo}"
async def load_assigned_pull_review():
if not await gitea_proxy.is_assigned_pull(repository, number):
raise HTTPException(status_code=404, detail="Assigned pull request not found")
return await gitea_proxy.pull_completion_review(repository, number)
capabilities = await _pull_workspace_capabilities(repository, number)
if not _has_pull_workspace_access(capabilities):
raise HTTPException(status_code=404, detail="Pull request not found")
review = await gitea_proxy.pull_completion_review(repository, number)
return {**review, "capabilities": capabilities}
try:
return await asyncio.wait_for(
@ -6690,8 +6710,10 @@ async def assigned_pull_review_feedback(
repository = f"{owner}/{repo}"
async def load_feedback():
if not await gitea_proxy.is_assigned_pull(repository, number):
raise HTTPException(status_code=404, detail="Assigned pull request not found")
if not _has_pull_workspace_access(
await _pull_workspace_capabilities(repository, number)
):
raise HTTPException(status_code=404, detail="Pull request not found")
return await gitea_proxy.pull_review_feedback(
repository, number, review_id, expected_head_sha
)
@ -6730,8 +6752,10 @@ async def assigned_pull_checks(
repository = f"{owner}/{repo}"
async def load_checks():
if not await gitea_proxy.is_assigned_pull(repository, number):
raise HTTPException(status_code=404, detail="Assigned pull request not found")
if not _has_pull_workspace_access(
await _pull_workspace_capabilities(repository, number)
):
raise HTTPException(status_code=404, detail="Pull request not found")
return await gitea_proxy.pull_check_status(repository, number)
try:
@ -6811,8 +6835,10 @@ async def pull_review_candidates(
) -> JSONResponse:
repository = f"{owner}/{repo}"
try:
if not await gitea_proxy.is_assigned_pull(repository, number):
raise HTTPException(status_code=404, detail="Assigned pull request not found")
if not _has_pull_workspace_access(
await _pull_workspace_capabilities(repository, number)
):
raise HTTPException(status_code=404, detail="Pull request not found")
result = await asyncio.wait_for(
gitea_proxy.pull_review_candidates(repository, number),
timeout=ISSUE_ACTION_TIMEOUT_SECONDS,
@ -6915,8 +6941,10 @@ async def assigned_pull_conversation(
repository = f"{owner}/{repo}"
async def load_conversation():
if not await gitea_proxy.is_assigned_pull(repository, number):
raise HTTPException(status_code=404, detail="Assigned pull request not found")
if not _has_pull_workspace_access(
await _pull_workspace_capabilities(repository, number)
):
raise HTTPException(status_code=404, detail="Pull request not found")
return await gitea_proxy.issue_conversation_page(repository, number, page, limit)
try:
@ -6944,8 +6972,10 @@ async def comment_on_assigned_pull(
repository = f"{owner}/{repo}"
async def post_comment():
if not await gitea_proxy.is_assigned_pull(repository, number):
raise HTTPException(status_code=404, detail="Assigned pull request not found")
if not _has_pull_workspace_access(
await _pull_workspace_capabilities(repository, number)
):
raise HTTPException(status_code=404, detail="Pull request not found")
return await gitea_proxy.comment_on_issue(repository, number, comment.body)
try:
@ -6986,8 +7016,8 @@ async def merge_assigned_pull(
)
try:
assigned = await asyncio.wait_for(
gitea_proxy.is_assigned_pull(repository, number),
capabilities = await asyncio.wait_for(
_pull_workspace_capabilities(repository, number),
timeout=ISSUE_ACTION_TIMEOUT_SECONDS,
)
except Exception:
@ -6996,8 +7026,8 @@ async def merge_assigned_pull(
status_code=503,
headers={"Retry-After": "1"},
)
if not assigned:
raise HTTPException(status_code=404, detail="Assigned pull request not found")
if not _has_pull_workspace_access(capabilities):
raise HTTPException(status_code=404, detail="Pull request not found")
journal = _security_event_store()
try:

View File

@ -26,6 +26,7 @@ def test_authored_pull_queue_is_phone_usable_and_opens_the_existing_pull_route(v
page.add_script_tag(path=FRONTEND / "my-work.js")
page.add_script_tag(path=FRONTEND / "work-route.js")
page.add_script_tag(path=FRONTEND / "mobile-queue-launcher.js")
page.add_script_tag(path=FRONTEND / "pull-sheet.js")
page.evaluate("document.querySelector('#mobile-queue-sheet').showModal()")
row = page.locator('[data-mobile-queue="authored"]')
@ -63,6 +64,25 @@ def test_authored_pull_queue_is_phone_usable_and_opens_the_existing_pull_route(v
"opened": "opened",
"count": 1,
}
capabilities = page.evaluate("""() => {
const item = {key:'stackchain/dashboard#41'};
createPullSheet.resetOwnershipControls(
document, item, () => false, {authored:true, assigned:false}
);
const ownership = document.querySelector('#pull-ownership');
return {
hidden: ownership.hidden,
inert: ownership.inert,
releaseDisabled: document.querySelector('#release-pull').disabled,
handoffDisabled: document.querySelector('#load-pull-handoff').disabled,
};
}""")
assert capabilities == {
"hidden": True,
"inert": True,
"releaseDisabled": True,
"handoffDisabled": True,
}
assert page.evaluate(
"document.documentElement.scrollWidth > document.documentElement.clientWidth"
) is False

View File

@ -7642,6 +7642,39 @@ process.stdout.write(JSON.stringify({{
assert output["released"] == "stackchain/api#7 released. Choose the next ready Today item."
def test_authored_unassigned_pull_hides_assignment_ownership_controls():
script = f"""
const createPullSheet = require({json.dumps(str(PULL_SHEET))});
const elements = new Map([
['#pull-ownership', {{open:true,hidden:false,inert:false}}],
['#pull-handoff-recipient', {{innerHTML:'stale',disabled:false}}],
['#confirm-pull-handoff', {{disabled:false,textContent:''}}],
['#load-pull-handoff', {{disabled:false}}],
['#release-pull', {{disabled:false,textContent:''}}],
['#pull-handoff-status', {{textContent:''}}],
['#edit-pull-content', {{hidden:false}}],
['#pull-edit-form', {{hidden:false}}],
['#pull-edit-status', {{textContent:'stale'}}],
]);
const doc = {{querySelector:selector => elements.get(selector)}};
createPullSheet.resetOwnershipControls(
doc,
{{key:'stackchain/api#7'}},
() => false,
{{authored:true,assigned:false}}
);
process.stdout.write(JSON.stringify(Object.fromEntries(elements)));
"""
output = json.loads(subprocess.run(
["node", "-e", script], check=True, capture_output=True, text=True
).stdout)
assert output["#pull-ownership"]["hidden"] is True
assert output["#pull-ownership"]["inert"] is True
assert output["#load-pull-handoff"]["disabled"] is True
assert output["#release-pull"]["disabled"] is True
def test_pull_sheet_single_flights_handoff_and_release_ownership_mutations():
script = f"""
const createPullSheet = require({json.dumps(str(PULL_SHEET))});

View File

@ -39,7 +39,7 @@ async def test_pull_author_can_publish_assigned_draft_at_expected_head(monkeypat
@pytest.mark.anyio
async def test_gitea_publishes_only_authored_assigned_draft_and_verifies_ready_head():
async def test_gitea_publishes_authored_unassigned_draft_and_verifies_ready_head():
requests = []
pull = {
"number": 7,
@ -48,7 +48,7 @@ async def test_gitea_publishes_only_authored_assigned_draft_and_verifies_ready_h
"draft": True,
"merged": False,
"user": {"login": "alex"},
"assignees": [{"login": "alex"}],
"assignees": [],
"head": {"sha": "abc1234"},
}
@ -131,7 +131,7 @@ async def test_pull_author_can_update_assigned_open_pull_context(monkeypatch):
@pytest.mark.anyio
async def test_gitea_pull_context_update_requires_author_assignment_and_matching_head():
async def test_gitea_pull_context_update_allows_author_without_assignment_at_matching_head():
requests = []
async def handler(request):
@ -141,7 +141,7 @@ async def test_gitea_pull_context_update_requires_author_assignment_and_matching
if request.url.path == "/api/v1/repos/stackchain/api/pulls/7" and request.method == "GET":
return httpx.Response(200, json={
"number": 7, "title": "Old context", "body": "Old body", "state": "open",
"merged": False, "user": {"login": "alex"}, "assignees": [{"login": "alex"}],
"merged": False, "user": {"login": "alex"}, "assignees": [],
"head": {"sha": "abc1234"},
"html_url": "https://forge.example/stackchain/api/pulls/7",
})
@ -172,6 +172,69 @@ async def test_gitea_pull_context_update_requires_author_assignment_and_matching
]
@pytest.mark.anyio
async def test_gitea_authored_unassigned_pull_has_workspace_access():
requests = []
async def handler(request):
requests.append((request.method, request.url.path))
if request.url.path == "/api/v1/user":
return httpx.Response(200, json={"login": "alex"})
if request.url.path == "/api/v1/repos/stackchain/api/pulls/7":
return httpx.Response(200, json={
"number": 7,
"state": "open",
"merged": False,
"user": {"login": "alex"},
"assignees": [],
})
raise AssertionError(f"unexpected request: {request.method} {request.url.path}")
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
try:
capabilities = await gitea_proxy.pull_workspace_capabilities("stackchain/api", 7)
finally:
await gitea_proxy.stop_client()
assert capabilities == {"authored": True, "assigned": False}
assert requests == [
("GET", "/api/v1/user"),
("GET", "/api/v1/repos/stackchain/api/pulls/7"),
]
@pytest.mark.anyio
async def test_authored_unassigned_pull_detail_returns_capabilities(monkeypatch):
async def assigned(repository, number):
return False
async def capabilities(repository, number):
assert (repository, number) == ("stackchain/api", 7)
return {"authored": True, "assigned": False}
async def detail(repository, number):
return {
"repository": repository,
"number": number,
"title": "Ship mobile flow",
"body": "Ready for review",
"author": "alex",
"head_sha": "abc1234",
"state": "open",
"conversation": {"comments": [], "page": 1, "older_page": None, "total": 0},
}
monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned)
monkeypatch.setattr(main.gitea_proxy, "pull_workspace_capabilities", capabilities)
monkeypatch.setattr(main.gitea_proxy, "pull_completion_detail", detail)
transport = httpx.ASGITransport(app=main.app)
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
response = await client.get("/api/v1/repos/stackchain/api/pulls/7/detail")
assert response.status_code == 200
assert response.json()["capabilities"] == {"authored": True, "assigned": False}
@pytest.mark.anyio
async def test_assigned_pull_detail_reports_completion_state(monkeypatch):
async def assigned(repository, number):
@ -213,7 +276,11 @@ async def test_assigned_pull_detail_rejects_unassigned_pull(monkeypatch):
async def assigned(repository, number):
return False
async def capabilities(repository, number):
return {"authored": False, "assigned": False}
monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned, raising=False)
monkeypatch.setattr(main.gitea_proxy, "pull_workspace_capabilities", capabilities)
transport = httpx.ASGITransport(app=main.app)
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
response = await client.get("/api/v1/repos/private/secret/pulls/9/detail")
@ -1208,7 +1275,7 @@ async def test_pull_handoff_candidates_are_bounded_and_exclude_invalid_or_curren
@pytest.mark.anyio
async def test_gitea_pull_review_request_filters_candidates_and_confirms_requested_reviewer():
async def test_gitea_authored_unassigned_pull_review_request_filters_candidates_and_confirms_reviewer():
requests = []
review_requested = False
@ -1230,8 +1297,8 @@ async def test_gitea_pull_review_request_filters_candidates_and_confirms_request
requested.append({"login": "casey"})
return httpx.Response(200, json={
"number": 7, "state": "open", "merged": False,
"head": {"sha": "abc123"}, "user": {"login": "alex"},
"assignees": [{"login": "timmy"}],
"head": {"sha": "abc123"}, "user": {"login": "timmy"},
"assignees": [],
"requested_reviewers": requested,
})
if request.method == "POST" and request.url.path.endswith("/requested_reviewers"):
@ -1248,7 +1315,10 @@ async def test_gitea_pull_review_request_filters_candidates_and_confirms_request
finally:
await gitea_proxy.stop_client()
assert candidates == [{"login": "casey", "name": "Casey"}]
assert candidates == [
{"login": "alex", "name": "Alexander"},
{"login": "casey", "name": "Casey"},
]
assert result == {
"repository": "stackchain/api", "number": 7, "head_sha": "abc123",
"requested_reviewers": ["sam", "casey"], "reviewer": "casey",
@ -1259,7 +1329,7 @@ async def test_gitea_pull_review_request_filters_candidates_and_confirms_request
@pytest.mark.anyio
async def test_gitea_cancel_pending_pull_review_confirms_authoritative_removal():
async def test_gitea_authored_unassigned_pull_can_cancel_pending_review():
requests = []
cancelled = False
@ -1271,7 +1341,7 @@ async def test_gitea_cancel_pending_pull_review_confirms_authoritative_removal()
if request.url.path.endswith("/pulls/7"):
return httpx.Response(200, json={
"number": 7, "state": "open", "merged": False,
"head": {"sha": "abc123"}, "assignees": [{"login": "timmy"}],
"head": {"sha": "abc123"}, "user": {"login": "timmy"}, "assignees": [],
"requested_reviewers": [{"login": "casey"}] if cancelled else [{"login": "sam"}, {"login": "casey"}],
})
if request.method == "DELETE" and request.url.path.endswith("/requested_reviewers"):

View File

@ -107,6 +107,36 @@ async def test_resolve_work_route_returns_only_a_requested_review():
}
@pytest.mark.anyio
async def test_resolve_work_route_returns_an_open_authored_unassigned_pull():
async def upstream(request):
if request.url.path.endswith("/user"):
return httpx.Response(200, json={"login": "timmy"})
return httpx.Response(200, json={
"id": 703,
"number": 90,
"title": "Ship authored flow",
"state": "open",
"html_url": gitea_proxy.GITEA_URL + "/stackchain/dashboard/pulls/90",
"assignees": [],
"requested_reviewers": [],
"user": {"login": "timmy"},
})
gitea_proxy.start_client(transport=httpx.MockTransport(upstream))
try:
result = await gitea_proxy.resolve_work_route(
"pull", "stackchain/dashboard", 90, None
)
finally:
await gitea_proxy.stop_client()
assert result["kind"] == "pull"
assert result["number"] == 90
assert result["is_assigned"] is False
assert result["work_reasons"] == ["authored_by_me"]
@pytest.mark.anyio
async def test_resolve_work_route_returns_an_open_issue_filed_by_the_current_user():
requests = []