Repository navigation
ci: put the workflows that only fire from the default branch on it - #168
Conversation
No scheduled run has ever happened in this repository. `runs schedule: 0`, from the API. security.yml and housekeeping.yml have ten runs each and every one of them is a push; nuget-watch.yml and prerelease.yml do not appear in the workflow list at all. The reason is the same for all four: GitHub fires `schedule` and `workflow_dispatch` only from the default branch, and these live on develop. Everything that runs on `push` or `pull_request` has been working, which is why it went unnoticed, and it means the weekly CodeQL scan, the stale sweep and the NuGet watch have all been decorative since they were written. Four files, and not the whole suite, because main is still the pre-rewrite tree: - prerelease.yml. Every job checks out the ref it is given, develop by default, so it reads nothing from main and only has to exist here to be dispatchable. - housekeeping.yml. Labels conflicts and closes stale threads through the API, touching no repository content. - security.yml. CodeQL builds with -p:JellyfinTarget=jf12, which is an unknown property against main's csproj and therefore ignored: main pins net9.0 and Jellyfin.Controller 10.11.*-* directly rather than through Directory.Build.props. A push to develop still scans develop, so both branches are covered. - nuget-watch.yml, with the ref it reads named rather than inherited, since what it compares NuGet against is decided in develop's Directory.Build.props. build.yml is deliberately left off. Its matrix builds jf11 and jf12 and then packages from bin/Release/<tfm>, and main has neither the target switch nor the bun tooling the pages job needs, so it would fail on every push here. It already runs on pull requests from the branch that carries it, which is where it matters. release.yml is untouched. Main's copy matches main's code and cutting a stable release is not what this changes. manifest-unstable.json comes along as an empty skeleton so the URL answers from the day the workflow can run, rather than 404ing until the first build. This is the branch model written into the plumbing: develop is the unstable channel and main is the stable one.
WalkthroughChangesAdds four GitHub Actions workflows for maintenance, NuGet monitoring, prerelease publishing, and security scanning. Adds a Streamyfin entry to Repository automation
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Merge Risk: 🟠 High · up to The new prerelease workflow can publish an incomplete or incorrect release and exposes repository write access to selected-ref scripts. These issues should be fixed before merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 5
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 19541080-44c4-42ee-9e63-996ada2cfe4d
📒 Files selected for processing (5)
.github/workflows/housekeeping.yml.github/workflows/nuget-watch.yml.github/workflows/prerelease.yml.github/workflows/security.ymlmanifest-unstable.json
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.
📜 Review details
🧰 Additional context used
🪛 zizmor (1.29.0)
.github/workflows/security.yml
[warning] 37-41: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 71-79: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 29-29: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
[warning] 68-68: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
.github/workflows/housekeeping.yml
[error] 6-14: use of fundamentally insecure workflow trigger (dangerous-triggers): pull_request_target is almost always used insecurely
(dangerous-triggers)
[warning] 26-26: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
[warning] 44-44: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
[warning] 6-14: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
.github/workflows/nuget-watch.yml
[warning] 42-44: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 33-33: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
.github/workflows/prerelease.yml
[warning] 42-45: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 69-72: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 123-126: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 95-95: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 102-102: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 103-103: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 104-104: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 117-117: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
🔇 Additional comments (2)
manifest-unstable.json (1)
1-12: LGTM!.github/workflows/nuget-watch.yml (1)
63-63: 🩺 Stability & AvailabilityThe workflow captures stdout from
node scripts/jellyfin-watch.jsat line 63 and parses it at line 74. However,develop:scripts/jellyfin-watch.jscould not be inspected because the checkout has nodevelopref, and no tracked script path was found. Its dependencies and stdout contract remain undecidable.
A line starting with #126 reads as a heading to markdownlint. The sentence about the two triggers conflated two things. Both need the file on the default branch to exist at all; from there schedule runs the default branch's copy while workflow_dispatch can be pointed at another branch, which is exactly how the first unstable build was published from develop. And the progress table says everything below is on develop, which #168 is not.
What this changes
Four workflow files and an empty manifest, onto
main. No plugin code, somainkeepsserving exactly the 0.68.1.0 it serves today.
Why
No scheduled run has ever happened in this repository. From the API:
nuget-watch.ymlandprerelease.ymldo not appear in the workflow list at all, andgh api .../workflows/prerelease.ymlanswers 404.One cause for all of it: GitHub fires
scheduleandworkflow_dispatchonly from thedefault branch, and these files live on
develop. Everything triggered bypushorpull_requesthas worked the whole time, which is exactly why nobody noticed. The weeklyCodeQL scan, the stale-thread sweep and the NuGet watch have been decorative since the day
they were written.
It is also what blocks the first unstable build:
prerelease.ymlcannot be dispatchedfrom anywhere until it is here.
This writes the branch model into the plumbing:
developis the unstable channel,mainis the stable one.What is ported, and what is not
prerelease.ymldevelopby default, so it reads nothing frommainand only has to exist here to be dispatchablehousekeeping.ymlsecurity.ymlnuget-watch.ymldevelop'sDirectory.Build.propsbuild.ymlbin/Release/<tfm>, andmainhas neither the target switch nor the bun tooling thepagesjob needs. It would fail on every push here, and it already runs on pull requests from the branch that carries itrelease.ymlmain's copy matchesmain's code, and cutting a stable release is not what this is forOn
security.yml, the one that looked risky: CodeQL builds with-p:JellyfinTarget=jf12, andmainhas noDirectory.Build.props. Checked rather thanassumed,
main's csproj pins<TargetFramework>net9.0</TargetFramework>andJellyfin.Controller 10.11.*-*directly, so the property is simply unknown and MSBuildignores it. The job builds
mainas the net9.0 project it is. A push todevelopstillscans
develop, so both branches stay covered.manifest-unstable.jsoncomes along as an empty skeleton, so the URL answers from the daythe workflow can run rather than 404ing until the first build.
How it was checked
the files.
main's csproj was read to confirm theJellyfinTargetproperty is inert there.maincarries noDirectory.Build.props, nopackage.json, nobun.lockand nobunfig.toml, which is what rulesbuild.ymlout.prerelease.ymlends withgit push origin HEAD:mainfor the manifest commit and a protected branch would fail itafter the tag and the release already existed. The only active ruleset on the default
branch is
deletion; there is no required review, no required check and no pushrestriction.
Nothing here can be proven by CI, since the whole point is triggers that only exist once
the file is on the default branch. The first scheduled run and the first dispatch are the
evidence, and both should be watched.
dotnet teston both targets, or CI says so — N/A, no code changesSummary by CodeRabbit
New Features
Maintenance