A few inconsistencies with GitHub's behavior:

1. actor vs triggering_actor semantics

On GitHub, actor is the person who originally triggered the workflow and stays constant across all attempts. triggering_actor is the person who initiated each specific attempt and changes on rerun. In this PR, ToActionWorkflowRun sets both fields to the attempt's trigger user.

Example from GitHub (cli/cli run 23574913305):

• Attempt 1: actor=mateenali66, triggering_actor=mateenali66
• Attempt 2: actor=mateenali66, triggering_actor=williammartin

The actor should remain run.TriggerUser regardless of which attempt is being viewed.

2. Missing previous_attempt_url field

GitHub's workflow run response includes previous_attempt_url which links to the API endpoint of the prior attempt (e.g. .../runs/123/attempts/1 for attempt 2, null for attempt 1). This field is absent from Gitea's ActionWorkflowRun struct.

3. Attempt switcher label differs from GitHub

On GitHub, the dropdown button shows just Latest when viewing the latest attempt. The PR currently shows Latest attempt #N. The dropdown items should show Attempt #N for all entries. Only the button text for the current latest should be just Latest.

This comment was written by Claude Opus 4.6.

/devtest/repo-action-view can also be updated, then more edge cases can be easily tested without really setting up an Actions run.

1. Would order buttons like [attempt] [re-run all]. People are use to buttons being at the same places.

2. Hide the button on first attempt, only show on subsequent ones (maybe that's already the case).

Attempt switcher label differs from GitHub

Fixed by 7c8e2a8. Please see the latest screenshots in description.

Race condition on concurrent reruns

9dafc83

RecreateTables on action_artifact in migration v331

d1d6b5c

actor vs triggering_actor semantics

8c5c5b9

Missing previous_attempt_url field

3d488fb

/devtest/repo-action-view can also be updated, then more edge cases can be easily tested without really setting up an Actions run.

Updated /devtest/repo-action-view in f354e00

/devtest/repo-action-view can also be updated, then more edge cases can be easily tested without really setting up an Actions run.

Once we have a auto-registered runner available during make watch or similar, actions testing will become easier (I will see to adding that in #37097, but not sure yet how to solve, ideally I'd like the user be able to specify the path to the runner repo).

/devtest/repo-action-view can also be updated, then more edge cases can be easily tested without really setting up an Actions run.

Once we have a auto-registered runner available during make watch or similar, actions testing will become easier (I will see to adding that in #37097).

I don't see it can be "easier" without the devtest page. Even if you ca have "auto-registered runner", how can you provide various different edge cases?

Devtest page is good and simple, but it shouldn't be a maintenance burden like it currently seems to be.

Devtest page is good and simple, but it shouldn't be a maintenance burden like it currently seems to be.

Show your proper approach to avoid such "maintenance burden" which can easily cover various edge cases and make developers can make changes to the related components and test the changes clearly.

Probably mock the runner on RPC level, give it fake job(s) to run, then render those fake jobs using the same template used on regular pages. Probably not easy if you also want edge case coverage.

Probably mock the runner on RPC level, give it fake job(s) to run, then render those fake jobs using the same template used on regular pages. Probably not easy if you also want edge case coverage.

Then what's the different from the current "devtest" page mocking? The current devtest page already provides the necessary mocking data. The template file is just one line {{template}} to include the component.

Probably mock the runner on RPC level, give it fake job(s) to run, then render those fake jobs using the same template used on regular pages. Probably not easy if you also want edge case coverage.

There are already a mock runner inside the integration tests.

Interesting.
devtest/repo-action-view is empty
but devtest/repo-action-view/runs/10 is not

Interesting. devtest/repo-action-view is empty but devtest/repo-action-view/runs/10 is not

I think this is because the run summary page can only work when the path contains a run_num (like /runs/10). The Run:CanRerunLatest and Run:PreviousAttempt link should work correctly.

I could swear it worked at some time... but yes I like

Code Review: PR #37119 — Introduce ActionRunAttempt

Reviewed by Claude Sonnet 4.6

Overview

This PR is a significant and well-motivated refactor. Introducing ActionRunAttempt as a first-class model — making each execution attempt explicit rather than mutating run/job records in-place — is the right direction. The plan-based rerun model (buildRerunPlan → execRerunPlan) is a clear architectural improvement.

Database & Migration

Positives:

• Unique constraint on action_artifact correctly updated to (RunID, RunAttemptID, ArtifactName, ArtifactPath) for per-attempt artifact isolation.
• LatestAttemptID on ActionRun is a clean approach for backward compatibility (0 = legacy run).

Concerns:

• Destructive schema change: ConcurrencyGroup / ConcurrencyCancel are removed from ActionRun and moved to ActionRunAttempt. This is irreversible — existing concurrency data is dropped. The migration should document this explicitly, and consideration should be given to whether old concurrency settings need backfilling into the new table.

• Large-table migration risk: Adding columns and updating the unique index on action_artifact may be very slow on large deployments with millions of artifact rows. No comment in the migration acknowledges this risk.

• No rollback strategy: Migration v331 makes multiple structural changes. If a deployment fails mid-migration, recovery is difficult.

Rerun Logic

Positives:

• Plan-based two-phase model is much cleaner than the old mutation-in-place approach.
• Separating buildRerunPlan (validation/planning) from execRerunPlan (side effects) is good engineering.
• SourceTaskID cleanly tracks "this job reuses an earlier attempt's result."

Concerns:

• Job matching for legacy runs is fragile (AttemptJobID == 0 path). Matching jobs by their positional index in a slice is brittle — if a workflow YAML was edited between runs and the job count changes, reruns on legacy jobs could silently match the wrong job with no error surfaced to the user.

• Silent failure edge case in buildRerunPlan: The validation that a job belongs to the template set happens in execRerunPlan, not buildRerunPlan. A plan can be "built" successfully but fail on execution — defeating the purpose of the two-phase model for that case.

• Missing atomicity guarantee: execRerunPlan performs multiple DB writes (create attempt, create jobs, update run). If a failure occurs mid-execution, the run could end up in a partially-updated state. The entire execute phase should be wrapped in an explicit DB transaction.

API

Positives:

• New endpoints /runs/{run}/attempts/{attempt} and /runs/{run}/attempts/{attempt}/jobs are well-scoped.
• 409 Conflict on duplicate attempt creation is correct behavior.
• PreviousAttemptURL in responses aids navigation.

Concerns:

• No pagination for attempt listing — if a run is retried many times, fetching all attempts could be unbounded. Consider adding standard pagination.

• The new ActionRunAttempt type should be verified against the GitHub Actions API schema if GH compatibility is intended, given the stated goal of aligning run_attempt semantics with GitHub.

Artifact Handling

Positives:

• Per-attempt artifact isolation is correct and important.
• ListUploadedArtifactsMetaByRunAttempt and SetArtifactNeedDeleteByRunAttempt cleanly scope operations to an attempt.

Concerns:

• EffectiveTaskID() usage — jobs now have both TaskID and SourceTaskID. If any call sites use raw TaskID instead of EffectiveTaskID(), skipped-job artifacts may resolve incorrectly. A pass over all call sites not using the effective accessor would be worthwhile.

• Legacy artifact access — artifacts with RunAttemptID = 0 are legacy. The code should be verified to ensure these are still accessible through the UI and API without requiring an attempt number.

UI / Templates

Positives:

• Attempt switcher dropdown with per-attempt status labels is a good UX addition.
• Read-only mode for historical attempts prevents accidental reruns on old attempts.

Concerns:

• "Latest Attempt" label logic — on a run with only one attempt (the common case), verify that no unnecessary attempt-navigation chrome is rendered.
• Artifact deletion URL with ?attempt=N — verify that this cannot inadvertently delete artifacts from a different attempt than the one being viewed.

Code Quality

• notify.go centralization is a good factoring change.
• NotifyWorkflowJobsAndRunsStatusUpdate batching is cleaner than scattered per-job notifications.
• GetEffectiveConcurrency() is an appropriate abstraction for handling both legacy and new concurrency fields.

Minor:

• buildRerunPlan and execRerunPlan are complex enough to warrant godoc comments.
• Please verify no trailing whitespace in new files.

Test Coverage

• Good: artifact isolation tests validate cross-attempt visibility rules.
• Good: rerun tests validate run_attempt context variable increments.
• Missing: explicit tests for the legacy-job positional matching fallback path, and for partial-failure scenarios in execRerunPlan.

Summary

Main items to address before merging:

1. execRerunPlan should be wrapped in a single DB transaction to prevent partial-state on failure.
2. Legacy job positional matching should either be hardened or clearly documented as a known limitation with a code comment.
3. The destructive removal of concurrency columns from ActionRun should be explicitly noted in the migration with a comment explaining what data is dropped.