Replace index with id in actions routes#36842
Conversation
d8bab42 to
7106d44
Compare
There was a problem hiding this comment.
Pull request overview
Migrates the repository web UI Actions run/job routes from index-based identifiers (runIndex/jobIndex) to stable database IDs (runID/jobID), aligning the web routes with the API and enabling more reliable addressing (especially with reusable workflows/child jobs).
Changes:
- Update frontend components/templates to pass and consume
runId/jobId, and generate/actions/runs/{runID}/jobs/{jobID}links. - Update backend web handlers and helpers to resolve runs/jobs by repo+ID (and run+job ID) and adjust log/rerun/cancel/approve/delete endpoints accordingly.
- Update and extend integration/unit tests to reflect ID-based URLs and add coverage for cross-repo run/job access protections.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| web_src/js/features/repo-actions.ts | Pass runId/jobId props into the Actions view Vue app. |
| web_src/js/components/WorkflowGraph.vue | Switch “current job” selection and job links from index to job ID. |
| web_src/js/components/RepoActionView.vue | Update API calls and UI links (jobs, logs, rerun) to use IDs. |
| templates/repo/actions/view_component.tmpl | Replace data-run-index/data-job-index with data-run-id/data-job-id. |
| templates/repo/actions/view.tmpl | Pass RunID/JobID into the view component instead of indexes. |
| templates/devtest/repo-action-view.tmpl | Update devtest template wiring to use RunID/JobID. |
| routers/web/repo/actions/view.go | Rework run/job resolution and all related endpoints (view/post/logs/rerun/cancel/approve/delete) to use IDs. |
| routers/web/devtest/mock_actions.go | Update devtest mock handler data keys to RunID/JobID. |
| routers/common/actions.go | Replace “download logs by job index” helper with “by job ID” helper. |
| routers/api/v1/repo/actions_run.go | Scope job-log download lookup by repo+job ID. |
| routers/api/v1/repo/action.go | Return Actions dispatch HTMLURL using run ID instead of run index. |
| services/convert/convert.go | Update webhook/API conversion to emit job HTMLURL using job ID. |
| services/actions/rerun.go | Ensure run attributes are loaded before rerun logic continues (needed for downstream usage). |
| services/actions/commit_status.go | Update commit-status target URLs and parsing/lookup paths to use IDs. |
| models/actions/run.go | Change ActionRun HTMLURL()/Link() to use run database ID. |
| models/actions/run_job.go | Replace GetRunJobByID with repo/run-scoped getters; adjust UpdateRunJob reload path accordingly. |
| models/actions/task.go | Ensure job loading and run-job updates are repo-scoped; include RepoID when updating jobs from tasks. |
| models/git/commit_status_test.go | Update Actions target URL test fixture to the new /runs/{id}/jobs/{id}-style pattern. |
| tests/integration/repo_webhook_test.go | Update expected webhook HTMLURL and UI endpoints to use run/job IDs. |
| tests/integration/api_actions_run_test.go | Update job lookup helpers in API rerun tests to be run-scoped. |
| tests/integration/actions_rerun_test.go | Update rerun endpoints to use run/job IDs. |
| tests/integration/actions_log_test.go | Update log download paths to use run/job IDs derived from tasks. |
| tests/integration/actions_delete_run_test.go | Resolve run ID from run number and update all URLs to use run/job IDs. |
| tests/integration/actions_concurrency_test.go | Update approve/rerun/cancel/job rerun URLs to use IDs; adjust assertions accordingly. |
| tests/integration/actions_route_test.go | New integration test validating repo scoping and 404 behavior for mismatched run/job IDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot <[email protected]> Signed-off-by: Zettat123 <[email protected]>
|
Since the legacy gitea/services/actions/commit_status.go Lines 190 to 197 in eb020a9 I’ve added a migration to convert all legacy TargetURLs into id-based URLs. This migration also addresses the issue in #36842 (comment) |
|
Review by Claude on behalf of @silverwind
|
* giteaofficial/main: Bound PageSize in `ListUnadoptedRepositories` (go-gitea#36884) Fix timeline event layout overflow with long content (go-gitea#36595) [skip ci] Updated translations via Crowdin Replace index with id in actions routes (go-gitea#36842) Enable eslint concurrency (go-gitea#36878)
…37008) Follow up #36842 Migration `326` can be prohibitively slow on large instances because it scans and rewrites all commit status target URLs generated by Gitea Actions in the database. This PR refactors migration `326` to perform a partial update instead of rewriting every legacy target URL. The reason for this partial rewrite is that **smaller legacy run/job indexes are the most likely to be ambiguous with run/job ID-based URLs** during runtime resolution, so this change prioritizes that subset while avoiding the cost of rewriting all legacy records. To preserve access to old links, this PR introduces `resolveCurrentRunForView` to handle both ID-based URLs and index-based URLs: - For job pages (`/actions/runs/{run}/jobs/{job}`), it first tries to confirm that the URL is ID-based. It does so by checking whether `{job}` can be treated as an existing job ID in the repository and whether that job belongs to `{run}`. If that match cannot be confirmed, it falls back to treating the URL as legacy `run index + job index`, resolves the corresponding run and job, and redirects to the correct ID-based URL. - When both ID-based and index-based interpretations are valid at the same time, the resolver **prefers the ID-based interpretation by default**. For example, if a repository contains one run-job pair (`run_id=3, run_index=2, job_id=4`), and also another run-job pair (`run_id=1100, run_index=3, job_id=1200, job_index=4`), then `/actions/runs/3/jobs/4` is ambiguous. In that case, the resolver treats it as the ID-based URL by default and shows the page for `run_id=3, job_id=4`. Users can still explicitly force the legacy index-based interpretation with `?by_index=1`, which would resolve the same URL to `/actions/runs/1100/jobs/1200`. - For run summary pages (`/actions/runs/{run}`), it uses a best-effort strategy: by default it first treats `{run}` as a run ID, and if no such run exists in the repository, it falls back to treating `{run}` as a legacy run index and redirects to the ID-based URL. Users can also explicitly force the legacy interpretation with `?by_index=1`. - This summary-page compatibility is best-effort, not a strict ambiguity check. For example, if a repository contains two runs: runA (`id=7, index=3`) and runB (`id=99, index=7`), then `/actions/runs/7` will resolve to runA by default, even though the old index-based URL originally referred to runB. The table below shows how valid legacy index-based target URLs are handled before and after migration `326`. Lower-range legacy URLs are rewritten to ID-based URLs, while higher-range legacy URLs remain unchanged in the database but are still handled correctly by `resolveCurrentRunForView` at runtime. | run_id | run_index | job_id | job_index | old target URL | updated by migration 326 | current target URL | can be resolved correctly | |---|---|---|---|---|---|---|---| | 3 | 2 | 4 | 1 | `/user2/repo2/actions/runs/2/jobs/1` | true | `/user2/repo2/actions/runs/3/jobs/4` | true | | 4 | 3 | 8 | 4 | `/user2/repo2/actions/runs/3/jobs/4` | true | `/user2/repo2/actions/runs/4/jobs/8` | true (without migration 326, this URL will resolve to run(`id=3`)) | | 80 | 20 | 170 | 0 | `/user2/repo2/actions/runs/20/jobs/0` | true | `/user2/repo2/actions/runs/80/jobs/170` | true | | 1500 | 900 | 1600 | 0 | `/user2/repo2/actions/runs/900/jobs/0` | false | `/user2/repo2/actions/runs/900/jobs/0` | true | | 2400 | 1500 | 2600 | 0 | `/user2/repo2/actions/runs/1500/jobs/0` | false | `/user2/repo2/actions/runs/1500/jobs/0` | true | | 2400 | 1500 | 2601 | 1 | `/user2/repo2/actions/runs/1500/jobs/1` | false | `/user2/repo2/actions/runs/1500/jobs/1` | true | For users who already ran the old migration `326`, this change has no functional impact. Their historical URLs are already stored in the ID-based form, and ID-based URLs continue to resolve correctly. For users who have not run the old migration `326`, only a subset of legacy target URLs will now be rewritten during upgrade. This avoids the extreme runtime cost of the previous full migration, while all remaining legacy target URLs continue to work through the web-layer compatibility logic. Many thanks to @wxiaoguang for the suggestions.
This PR migrates the web Actions run/job routes from index-based
runIndexorjobIndexto database IDs.Improvements of this change:
jobIndexdepended on list order, making it hard to locate a specific job. UsingjobIDprovides stable addressing.