Skip to content

Add GET /repos/{owner}/{repo}/actions/workflows/{workflow_id}/runs#37196

Open
bn-zr wants to merge 16 commits intogo-gitea:mainfrom
bn-zr:feature/add-api-workflowruns
Open

Add GET /repos/{owner}/{repo}/actions/workflows/{workflow_id}/runs#37196
bn-zr wants to merge 16 commits intogo-gitea:mainfrom
bn-zr:feature/add-api-workflowruns

Conversation

@bn-zr
Copy link
Copy Markdown

@bn-zr bn-zr commented Apr 13, 2026

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 13, 2026
@bn-zr bn-zr marked this pull request as ready for review April 13, 2026 17:45
@bircni bircni requested a review from silverwind April 13, 2026 18:09
@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 13, 2026

This PR aims for GitHub API compatibility. A few items drift from that — implement what's feasible.

  1. workflow_id numeric IDs. GitHub docs:

    The ID of the workflow. You can also pass the workflow file name as a string.

    Gitea stores workflow_id as a filename (models/actions/run.go:36) with no numeric ID concept. Implementing this is out of scope, but the swagger description should say "filename only (e.g. build.yml)" so users don't pass numeric IDs expecting them to work.

  2. Missing query parameters. GitHub documents these; consider adding the feasible ones:

    • created — date-time range. Parse >=, <=, .. operators and add a created_unix range filter to FindRunOptions.
    • exclude_pull_requests — boolean. When true, add builder.Neq{"trigger_event": "pull_request"} in FindRunOptions.ToConds().
  3. Nonexistent workflow returns 200 with empty list. ActionsGetWorkflow in this same file returns 404 for an unknown workflow_id. For consistency:

    workflowID := ctx.PathParam("workflow_id")
    if _, err := convert.GetActionWorkflow(ctx, ctx.Repo.GitRepo, ctx.Repo.Repository, workflowID); err != nil {
        if errors.Is(err, util.ErrNotExist) {
            ctx.APIError(http.StatusNotFound, err)
        } else {
            ctx.APIErrorInternal(err)
        }
        return
    }

    Replace the empty-list test assertion with a 404 check.

  4. Test loop break. If the expected run appears first, filter checks on later runs are skipped. Drop the break:

    for _, run := range runList.Entries {
        verifyWorkflowRunCanbeFoundWithStatusFilter(t, workflowRunsURL, token, run.ID, "", run.Status, "", "", "", "")
        verifyWorkflowRunCanbeFoundWithStatusFilter(t, workflowRunsURL, token, run.ID, "", "", "", run.HeadBranch, "", "")
        verifyWorkflowRunCanbeFoundWithStatusFilter(t, workflowRunsURL, token, run.ID, "", "", run.Event, "", "", "")
        if run.ID == expectedRunID {
            found = true
        }
    }
  5. Filter coverage. Add actor and head_sha checks to testAPIWorkflowRunsByWorkflowID, mirroring testAPIWorkflowRunBasic.


Comment written by Claude Opus 4.6.

- Update workflow_id swagger description to clarify filename-only
- Add exclude_pull_requests query parameter
- Return 404 for nonexistent workflow in ActionsListWorkflowRuns
- Remove break from test loop so all runs are verified
- Add actor and head_sha filter coverage to testAPIWorkflowRunsByWorkflowID
- Update nonexistent workflow test to expect 404
@bircni bircni force-pushed the feature/add-api-workflowruns branch from c1fd337 to e15b1c2 Compare April 13, 2026 18:56
@bircni
Copy link
Copy Markdown
Member

bircni commented Apr 14, 2026

passes locally @silverwind can you rerun? thats weird...

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 15, 2026

Ensure you run without test cache: go test -count=1 ..., could also be flaky. Run a few times to verify that.

@lunny lunny added the type/enhancement An improvement of existing functionality label Apr 20, 2026
@silverwind
Copy link
Copy Markdown
Member

Is it ready?

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 20, 2026
@lunny lunny added this to the 1.27.0 milestone Apr 20, 2026
@lunny lunny added the topic/gitea-actions related to the actions of Gitea label Apr 20, 2026
silverwind and others added 2 commits April 21, 2026 00:54
Fix false 404s from ActionsListWorkflowRuns: the previous check returned
404 whenever no runs existed, so a valid workflow that hadn't been
triggered yet was reported as missing. Accept either existing runs or a
workflow file on the default branch as proof of existence; run lookup
runs first since it's cheaper than parsing workflow YAML on every call.

Also use the HookEventPullRequest constant instead of a raw string
literal for the trigger_event filter, switch the new
exclude_pull_requests query param to ctx.FormBool, and add a test
asserting exclude_pull_requests filters out pull_request-event runs.

Co-Authored-By: Claude (Opus 4.7) <[email protected]>
@silverwind
Copy link
Copy Markdown
Member

Cleaned up and addressed outstanding review in 897e97e.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 20, 2026
Comment thread models/actions/run_list.go Outdated
}
if opts.ExcludePullRequests {
cond = cond.And(builder.Neq{"`action_run`.trigger_event": string(webhook_module.HookEventPullRequest)})
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it really match GitHub's document?

If I understand correctly, ExcludePullRequests means "no PRs in the response", but doesn't mean filtering the runs.

Image

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, original review was a hallucination. Fixing it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to be right.

I really hope maintainers can be more careful for the AI outputs, don't blindly trust them.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review was from lobotomized Opus 4.6, sorry. And yes we need to be more careful.

Copy link
Copy Markdown
Member

@silverwind silverwind Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this seems to considerably increase this PR's scope, not sure it's worth. I'll push it, then we can decide whether to keep it or omit that flag for now.

Copy link
Copy Markdown
Member

@silverwind silverwind Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 1e0262f. @lunny you should also re-review please for this PullRequestMinimal introduction.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed the right meaning of ExcludePullRequests

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Apr 21, 2026
@silverwind silverwind self-requested a review April 21, 2026 01:15
The previous implementation interpreted exclude_pull_requests=true as
"filter out runs whose trigger event is pull_request", but GitHub's
parameter empties the pull_requests field on each run object instead —
the runs themselves are always returned.

Add a pull_requests field to ActionWorkflowRun matching GitHub's
minimal schema (id, number, url, head{ref,sha,repo{id,url,name}},
base{…}). Populate it in ToActionWorkflowRun from the PR that triggered
the run (pull_request / pull_request_review events) or from open PRs
whose head branch matches the push ref (push events). When the list
endpoint receives exclude_pull_requests=true, the flag short-circuits
the PR lookup so the field stays empty.

The removal of ExcludePullRequests from FindRunOptions undoes the wrong
trigger_event filter.

Co-Authored-By: Claude (Opus 4.7) <[email protected]>
@silverwind silverwind force-pushed the feature/add-api-workflowruns branch from 615047f to 1e0262f Compare April 21, 2026 01:27
@silverwind silverwind requested a review from lunny April 21, 2026 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged topic/gitea-actions related to the actions of Gitea type/enhancement An improvement of existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants