* Update docs-impact-review workflow with improved configuration
- Include PR head SHA in concurrency group to avoid cancelling concurrent runs on different commits
- Move permissions to workflow level
- Remove Docs/Not Needed label skip condition from job if clause
- Upgrade model to claude-sonnet-4-20250514 with increased max-turns (30→50)
- Add Bash(ls*) to allowedTools
- Add bug-fix pattern to no-docs-needed list
- Refine security fix rule to skip docs unless externally observable contract change
- Guard label addition behind !analysisFailed check
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Revert structural changes from #37084, keep prompt improvements
Reverts the concurrency group SHA addition, permissions restructure,
model/max-turns/tool changes, and label guard from #37084 as they are
not needed.
Keeps only the meaningful prompt changes:
- Remove Docs/Not Needed label skip from job if condition
- Add bug-fix pattern to no-docs-needed list
- Refine security fix rule to skip docs unless externally observable contract change
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Restore Docs/Not Needed label skip condition
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Update docs-impact-review workflow
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* Add CI check to prevent renumbering or renaming existing DB migrations
Backporting or rebasing a PR can accidentally change the sequence number
or description of a migration that already shipped on master, which
corrupts the upgrade path for existing installations (see MM-68848).
This adds a migration_check tool that compares the working tree's
migration files against the base branch and fails when an existing
migration's version or name differs. Brand new migrations are ignored.
It is wired into the existing "Check migration files" CI job via a new
`make check-migration-changes` target.
Co-authored-by: mattermost-code <matty-code@mattermost.com>
* Apply go fix strings.SplitSeq in migration_check
Co-authored-by: mattermost-code <matty-code@mattermost.com>
* Retrigger CI after transient Enterprise npm failure
Co-authored-by: mattermost-code <matty-code@mattermost.com>
* Use github.ref_name as migration check base ref fallback on push
Co-authored-by: mattermost-code <matty-code@mattermost.com>
* Flag shipped migrations missing from branch by version and name
Add reverse comparison so deletions and simultaneous renumber+rename are
caught, per CodeRabbit review feedback.
Co-authored-by: mattermost-code <matty-code@mattermost.com>
* Address PR feedback: 0 answered, 2 resolved, 0 declined
* Address PR feedback: 2 answered, 1 resolved, 1 declined
* Fix migration check direction and scope to release branches
Address isacikgoz review feedback:
- Reverse the comparison so it flags migration files the branch HAS that
the base branch (origin/master) does NOT, instead of flagging base
migrations missing from the branch. A backport branch is an older subset
of master, so the old direction reported every newer master migration as
an error.
- Include the up/down kind in the comparison key so a one-sided rename of
only the .up or .down file is no longer masked by its surviving partner.
- Restrict the CI step to PRs targeting release-* branches, where the head
is expected to be a subset of master; on master-targeted PRs new
migrations are normal and would otherwise be flagged as stray.
* Scope migration check to PR-added Postgres migrations
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: mattermost-code <matty-code@mattermost.com>
* Skip docs impact review when Docs/Not Needed label is present
Add a gate job that checks for the Docs/Not Needed label before running
the Claude analysis, so maintainers can opt out without the workflow
re-adding Docs/Needed on subsequent CI runs.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Simplify docs/not-needed gate using contains() expression
Replace the dedicated check-docs-not-needed job with a contains() check
on github.event.pull_request.labels in the docs-impact-review job's if:
condition. This avoids spinning up a separate runner and an extra API
call to achieve the same opt-out behavior.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
* Fix flaky table conversion for colspan section rows
The mikepenz flaky_summary HTML contains per-suite section-header rows
(<td colspan="2"><strong>...</strong></td>) that markdown tables cannot
represent. The sed pipeline only matched bare <td>/<th> with text-only
content, so those rows leaked raw HTML and broke the rendered table in
Mattermost.
Replace the sed/awk conversion with an inline python3 HTML parser that keeps
only rows matching the header column count (dropping the section-header
rows), unescapes HTML entities, escapes in-cell pipes, and emits a flat
markdown table.
Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>
* Collapse whitespace in flaky table cells
str.strip() only trims leading/trailing whitespace, so an embedded newline
(including one decoded from an entity like ) would remain and break the
single-line markdown table row. Collapse all internal whitespace to single
spaces when rendering each cell.
Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
* Fix broken migration/codegen CI self-checks
The git status self-checks in server-ci.yml became silent no-ops once
these jobs moved into the build container (#33679): the resolved shell
is sh, where the bash-only [[ ]] errors out, and git rejects the
checkout with "dubious ownership". Switch to POSIX [ ], mark the
workspace safe so git status runs, and print the diff on failure across
all affected checks.
* Regenerate stale migrations.list
The broken check-migrations step let an outdated migrations.list ship.
Regenerate it with make migrations-extract to add migration 000193.
* make mocks
* make gen-serialized
* make mmctl-docs
* Switch Cypress to use shared ESLint config
* Run --fix in Cypress
* Manually fix remaining lint issues in Cypress
* Switch Playwright to use shared ESLint config
* Run --fix in Playwright
* Manually fix remaining lint issues in Playwright
* Install and cache web app deps during Cypress CI builds
This also caches the types and client package. That isn't needed currently
since it uses prepackaged versions of those, but I imagine we might change
that at some point.
* Run e2e-tests-check when ESLint plugin is updated
* Change E2E test GHA caching to cache all of web app node_modules
* Fix mismatch between cache save and restore
* Try bumping cache keys
* Copy step to install dependencies to server.run_cypress.sh
I don't know how this must've worked before, but if this fixes the issue,
it seems like neither Cypress nor Playwright actually use the cached
depenendencies.
* Try disabling caching entirely for Cypress tests
* Try bypassing makefile?
* Try also manually building dependencies in run_specs.sh
I don't know why this appears to duplicate run_cypress.sh and
run_playwright.sh, both of which are called run_test.sh which
might not be used any more as best I can tell.
* Try installing the web app dependencies in yet another place
* Disable the extra steps in server.prepare.sh specifically for Cypress
* Revert changes to update cache key and disable web app depenedency cache on Cypress builds
* Move flaky test report from PR comment to Mattermost channel
Replace the github-script step that posted the flaky test summary as a PR
comment with a step that posts the summary to a Mattermost channel via a new
Mattermost incoming webhook (WEBHOOK_URL_FLAKY_TEST_MM). The HTML <table>
summary is converted to a Markdown table, with content pipes escaped and HTML
entities decoded. The existing custom flaky-test hub webhook is left untouched.
Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>
* Hoist github.server_url into SERVER_URL env var
Keep all GitHub Actions context expressions in the step's env block for
consistency, and build PR_URL purely from shell variables.
Co-authored-by: Maria A Nunez <maria.nunez@mattermost.com>
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
* Generate default_roles_permissions.js from a live server snapshot
Replace the hand-maintained default_roles_permissions.js with a
generated file. The generator (scripts/default_permissions_generator/)
boots a real app.NewServer() against a temporary database created by
storetest.MakeSqlSettings, lets all schema and app-level migrations run
naturally, then snapshots the resulting Roles table into the JS file.
This eliminates the drift risk of the previous hand-maintained map:
new migrations are picked up automatically because the same server
initialization path is used here as in production.
The CI job gets a postgres service and sets IS_CI=true so the generator
uses the same host-switching logic as the rest of the test suite.
Locally, make start-docker provides the postgres instance.
* Address CodeRabbit feedback: pin checkout SHA, add permissions, add .PHONY entries
* Remove build-opensearch-image.yml -- the published image is unused
* MM-68248: Support OpenSearch v3
* MM-68248: Add CI job to test OpenSearch v2 backwards compatibility
* MM-68248: Handle missing indexes gracefully before reindex
OpenSearch v3 rejects _update_by_query and _delete_by_query with no index
argument (405), and returns index_not_found_exception (404) when querying
an exact index name that hasn't been created yet. Both arise before any
reindex has run, since indexes are created on first document write.
Return nil/empty instead of an error from all affected operations, and add
test coverage for each in the no-indexes state.
* MM-68248: Fix copy-paste operation names in DeleteFilesBatch
* MM-68248: Add i18n string for delete_files_batch error
* Revert "MM-68248: Add i18n string for delete_files_batch error"
This reverts commit e885678088.
* Revert "MM-68248: Fix copy-paste operation names in DeleteFilesBatch"
This reverts commit 4b7caacf59.
* Revert "MM-68248: Handle missing indexes gracefully before reindex"
This reverts commit 2d2d522f86.
* allow workflow_dispatch trigger for Server CI (for plugins CI)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* [MM-68402] MBE Phase 2: declare four generic plugin hooks (#36291)
* new hooks-only phase 2
* remove ChannelWillBeMoved
* remove RecapWillBeProcessed and MessageWillBeRewrittenByAI
Drop the AI/recap hooks from the new-hook surface; AI-LLM paths
remain uncovered in tech preview and are documented as residuals.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [MM-68403] MBE Phase 3: ChannelGuards primitive (storage + cache + plugin API) (#36365)
* phase 3
* phase 3: register ChannelGuard mock in test setup helper
NewChannels' startup-time call to reloadGuardCache invokes
s.ChannelGuard().GetAll(); without an expectation on the mock store,
every test that sets up the server with GetMockStoreForSetupFunctions
panics during init.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* phase 3: register ChannelGuard mock in retrylayer test
retrylayer.New walks every store getter to wrap it; without the mock
expectation on ChannelGuard, TestRetry panics during layer construction.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* use rctx properly in the store methods
* phase 3: match rctx arg in testlib ChannelGuard mock
GetAll now takes request.CTX, so the testify expectation must include
mock.Anything; otherwise the call panics under the mocked store.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* phase 3: set api.ctx in TestChannelGuardLowercaseNormalization
The test constructs PluginAPI directly without a ctx, which used to
work when App.RegisterChannelGuard built its own EmptyContext. Now
that the App methods take rctx from the caller, the nil ctx panics
inside RequestContextWithMaster.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [MM-68404] MBE Phase 4: App-layer plugin hook wiring (#36407)
* phase 4
* Fix nil rctx in TestChannelGuardLowercaseNormalization
The PluginAPI struct literal was missing ctx: rctx after a refactor
moved the rctx declaration below the struct construction, leaving
api.ctx as nil. This caused a nil pointer dereference in reloadGuardCache
when RegisterChannelGuard called store.RequestContextWithMaster(nil).
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
* Remove ChannelWillBeMoved hook call from MoveChannel (phase 4)
The hook and its ID were removed from mbe-phase-2 but the call site in
MoveChannel and its i18n string were not cleaned up during the rebase.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* remove channel will be moved test
* Remove RecapWillBeProcessed and MessageWillBeRewrittenByAI hook calls (phase 4)
The hooks and their IDs were removed from mbe-phase-2 but the call sites
in ProcessRecapChannel and RewriteMessage, their i18n strings, and their
tests were not cleaned up during the rebase.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Revert channel_id plumbing on rewrite endpoint (phase 4)
The channel_id field on RewriteRequest was added in phase 4 to feed the
synthetic post passed to MessageWillBeRewrittenByAI. With that hook
removed from mbe-phase-2, channel_id has no consumer; revert the field,
the api4 validation, the app.RewriteMessage parameter, and the
corresponding webapp client + hook plumbing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [MM-68555] MBE Phase 5: Channel-guard enforcement + two-phase dispatch (#36473)
* phase 5
* Bake plugin counter-file paths into source instead of env vars
t.Setenv panics when an ancestor test calls t.Parallel, so the two
channel-guard tests broke under ENABLE_FULLY_PARALLEL_TESTS in CI.
Build each plugin source per-subtest with its temp file path embedded
as a Go literal — same pattern as TestPluginUploadsAPI.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Remove guarded helpers and tests for dropped hooks (phase 5)
The runGuardedRecapWillBeProcessed and runGuardedMessageWillBeRewrittenByAI
helpers were never wired (their app-layer call sites were already removed
in the phase-4 cleanup), and the corresponding sub-tests across panic /
allow / reject / partial plugins reference hooks that no longer exist.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [MM-68405] MBE Phase 6: fire MessagesWillBeConsumed on the edit path (#36475)
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* rebase onto master
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Create config-change-checker.yml
* Create check_config_changes_ci.py
* Update config-change-checker.yml
* Update check_config_changes_ci.py
* Update check_config_changes_ci.py
* Update check_config_changes_ci.py
* Update check_config_changes_ci.py
* Update config-change-checker.yml
* Update check_config_changes_ci.py
* Update config-change-checker.yml
* Update config.go
* Fix check_api to detect multi-line and multi-method endpoints
The previous implementation matched the .Handle(...).Methods(...) regex
line-by-line against diff lines. This silently missed two real and
common patterns in api4/:
1. Multi-line .Handle(...) declarations — e.g. group.go has 18 of
them, where the path lives on one line and the wrapper/handler on
the next. The regex never matched, so PRs adding such endpoints
produced empty release-note entries.
2. Multi-method declarations like
.Methods(http.MethodGet, http.MethodHead) (4 instances in file.go)
— the old regex required a closing paren immediately after the
first method.
The fix:
- Add a file_at(ref, path) helper that snapshots a file at a git ref
via 'git show', so checkers can compare full file states instead of
pattern-matching diff text.
- Add _scan_endpoints() that whitespace-collapses the file before
matching, letting the regex span what were originally multiple
lines.
- Loosen _HANDLE_RE to capture the methods list as a substring and
extract individual HTTP verbs with a known-method allowlist, so
multi-method declarations produce one entry per verb.
- Switch check_api to set-diff (after - before) / (before - after)
on the parsed endpoint sets. This also cleanly handles routes
that move within a file (no fragile add/remove dedup needed).
- Anchor the new/deleted file detection to '^new file mode \d+' to
avoid false positives from stray text in source files.
Made-with: Cursor
* Track enclosing struct in check_config to avoid dedup collisions
The previous check_config keyed its add/remove dedup on the bare field
name. The dedup intent was to ignore fields that were merely reordered
within config.go (which appear in the diff as both '-Foo' and '+Foo').
But because the key was just the field name, an unrelated rename in one
struct could silently cancel out a real new field with the same name in
a different struct. For example, in a single PR:
- EnableFoo *bool // removed from ServiceSettings
+ EnableFooV2 *bool
- EnableBar *bool // removed from EmailSettings
+ EnableFoo *bool // newly added — but wrongly cancelled below
The dedup would see 'EnableFoo' in both lists and drop both entries,
hiding the brand-new EmailSettings.EnableFoo from the release-note
output.
The fix tracks each field's enclosing struct using a brace-depth stack
that walks the file at BASE_SHA and HEAD_SHA. Fields are keyed as
(struct_name, field_name) tuples, so identically-named fields in
different structs are distinct, and the dedup only collapses true
reorderings. As a side benefit the rendered output is now
'StructName.FieldName' which is much more useful to reviewers.
Switching to file-at-revision scanning + set diff also removes the
custom dedup logic entirely — set arithmetic handles "moved within
file" naturally.
Made-with: Cursor
* Switch remaining checkers to file-at-revision style; drop lines_by_sign
check_audit_events and check_go_version still parsed +/- diff lines
directly, with the same brittle dedup-and-cancel logic that was used in
the previous check_config. After the previous two commits the rest of
the file uses the file_at(ref, path) helper to compare full file
states between BASE_SHA and HEAD_SHA, which:
- removes the entire moved-within-file dedup dance (set arithmetic
handles it for free),
- aligns all four checkers on a single, easy-to-reason-about pattern,
- is robust to whitespace-only or reordering edits in the watched
files.
For Dockerfile.buildenv the helper also avoids a subtle case where the
old code only inspected +/- lines: an edit to an unrelated RUN line
that didn't touch the FROM line could in theory leave both old_ver and
new_ver as None even though the version was effectively unchanged.
Reading the file at each revision compares the actual current and
previous FROM line directly.
The lines_by_sign helper now has no callers, so remove it.
Made-with: Cursor
* Update config.go
* Update config.go
* Update check_config_changes_ci.py
* Update check_config_changes_ci.py
* Update check_config_changes_ci.py
* Update check_config_changes_ci.py
* Tighten check_config_changes_ci.py: regex coverage + idempotency
- Restore tolerant `_HANDLE_RE` so 2-arg wrappers (e.g. `api.APISessionRequired(handler, handlerParamFileAPI)`)
are not silently dropped from the api4 endpoint scan; broaden the `.Methods(...)`
capture so string-literal variants (`Methods("GET")`) work too. Filtering moves
back to the `_HTTP_METHODS` allowlist in `_parse_methods` to keep stray
identifiers from being treated as HTTP verbs.
- Make `strip_old_note` also remove auto-generated lines that landed outside
the ```release-note fence (the inject_note fallback paths) so reruns no
longer accumulate duplicates when a PR has no fence.
- Skip the GitHub PATCH when the PR description is already up to date, so
every commit no longer triggers an unconditional write.
- Wire up `check_go_version`'s `additions` path in `_format_lines` and
`_AUTO_LINE_RE` so a freshly-added Dockerfile.buildenv emits a note.
- Remove the now-dead `CheckResult.to_markdown` method (replaced by
`_format_lines`).
Made-with: Cursor
* Restore ExperimentalSettings.EnableWatermark
The field was removed in f71527f0b1 but `server/config/client.go`,
`server/config/client_test.go`, and `server/public/model/config_test.go`
still reference it (added on master in #36025). Restoring the field
makes the branch compile again so CI can go green.
Made-with: Cursor
* Replace placeholder release-note content (NONE / N/A) on injection
The script previously appended its auto-detected lines INSIDE the
```release-note fence but never displaced template placeholders, so PRs
that only had `NONE` ended up with output like:
NONE
Added `Foo.Bar` configuration setting.
Go runtime updated from 1.25.8 to 1.25.9.
When the existing fence content is empty or consists only of placeholder
tokens (NONE, N/A, NA, dashes — case-insensitive), replace it entirely
with the auto-detected entries. User-written human content is still
preserved by appending instead.
Idempotent: stripping followed by re-injection keeps the placeholder
visible when there's nothing to inject, and replaces it again when there
is.
Made-with: Cursor
* Update config-change-checker.yml
* Update check_config_changes_ci.py
---------
Co-authored-by: Your Name <eva.sarafianou@gmail.com>
Co-authored-by: Mattermost Build <build@mattermost.com>
Workers no longer run `npm ci` — `node_modules` and framework binaries
are restored from actions/cache populated once by a new `prep-deps` job.
This closes the intermittent EEXIST/ENOENT failure inside npm's own
cacache writer that occasionally fails `npm ci` on a runner. Removing
`npm ci` from workers also cuts ~5 min of duplicated install work per worker.
dispatch-begin now runs as its own job after prep-deps so it fires once the
per-worker test-server setup is the only remaining work before dispatch-run.
* MM-68149: upgrade to Go 1.26.2
Update go directive in go.mod and .go-version.
* MM-68149: replace pointer helpers with Go 1.26 new()
Go 1.26 extends the built-in new() to accept an initial value expression,
making typed-pointer helpers like model.NewPointer(x), bToP(x), and boolPtr(x)
redundant. Replace every call site with new(x) and remove the now-unused
helper functions and their //go:fix inline directives.
* MM-68149: apply go fix for reflect API and format-string changes
- reflect.Ptr → reflect.Pointer (renamed in Go 1.18, deprecated alias removed in 1.26)
- reflect range-over-struct: for i := 0; i < t.NumField(); i++ → for field := range t.Fields()
and the equivalent for Methods() and interface types
- Fix format-string concatenation and variadic-arg mismatches flagged by go vet
* MM-68149: update JPEG fixtures and test infrastructure for Go 1.26 encoder
Go 1.26 ships a new image/jpeg encoder that produces slightly different output.
Regenerate all JPEG fixture files and switch the comparison helpers from
byte-equality to pixel-level comparison with a small per-channel tolerance,
so minor encoder drift across patch versions is handled automatically.
Add -update-fixtures flag to make it easy to regenerate fixtures after future
major Go upgrades. Document the update procedure in tests/README.md.
* MM-68149: CI check that go fix ./... produces no changes
* Fix real bugs flagged by CodeRabbit review
- group.go: set newGroup.MemberCount not group.MemberCount (member count
was populated on the wrong variable and lost before publish/return)
- file_test.go: guard compareImage(GetFilePreview) on the preview slice
length, not the thumbnail slice length (copy-paste error)
- config_test.go: remove duplicate MinimumLength assignment
* fixup! Fix real bugs flagged by CodeRabbit review
Only comment when action-junit-report reports no failing tests after retry
merging, so all-failure retries are not labeled flaky.
Remove skip/JIRA guidance from the comment body and use neutral wording
for the workflow run link.
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
* Update E2E test workflows to use context names and server images and bump playwright workers to 10
* refactor: update branch naming conventions in E2E test workflows for better aggregation
* change retry to 1, fixed and disabled failed tests
* add v2 templates for Cypress and Playwright E2E tests with test system io integration
* add commenting to pr
* identify more playwrights to fix separately
* disable deletion-report.spec for separate fix
---------
Co-authored-by: Mattermost Build <build@mattermost.com>
Restore the `fullyparallel: false` override for the unsharded
`Postgres with binary parameters` and `Postgres FIPS` jobs in the
weekly workflow. The override was originally added to the binary
parameters job in #35995 to prevent resource exhaustion on a single
runner, but was dropped when both jobs moved into
server-ci-weekly.yml in #36036, leaving them on the template default
of `true`.
Without it, the hosted runner is overwhelmed (too many server
instances, WebSocket hubs, and DB connections) and the runner agent
itself loses communication with GitHub mid-run, surfacing as
"hosted runner lost communication with the server" at ~55-60 min
into the Run Tests step. Both runs on April 27 and May 4 failed
this way; the sharded FIPS variant retained for FIPS-touching PRs
in server-ci.yml is unaffected because each shard handles only a
fraction of the packages.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Coverage job ran the same tests as Postgres with only ENABLE_COVERAGE=true
and a Codecov upload as differences. Enable coverage directly on the Postgres
job under the same release-branch skip condition, eliminating 4x 8-core runner
hours per PR.
* ci: fix startup_failure in nightly race and weekly workflows
Add id-token: write permission to both server-ci-nightly-race.yml and
server-ci-weekly.yml. The reusable server-test-template.yml declares
id-token: write in its permissions block (needed for FIPS Docker Hub
login via OIDC). GitHub requires that caller workflows grant at least
the permissions declared by any reusable workflow they invoke —
regardless of whether the steps using those permissions are skipped at
runtime. Both new workflows only declared contents: read, causing
immediate startup_failure with zero jobs created.
Release Note
```release-note
NONE
```
Co-authored-by: Claude <claude@anthropic.com>
* ci: remove unused id-token: write from server-test-template
The template declared id-token: write but nothing in the workflow uses
OIDC token exchange — the FIPS Docker Hub login uses plain secrets
(DOCKERHUB_USERNAME/DOCKERHUB_TOKEN), not an OIDC identity token.
Removing it from the template means caller workflows (nightly race,
weekly, and the main server-ci) no longer need to grant id-token: write
either, following the principle of least privilege.
This is the actual root cause fix: the previous commit added id-token
to the callers as a workaround, but the real issue was the template
requesting a permission it never uses.
Co-authored-by: Claude <claude@anthropic.com>
---------
Co-authored-by: Claude <claude@anthropic.com>
* ci: add yamllint workflow to detect duplicate YAML keys
Add a yamllint check for workflow files to catch duplicate keys that
YAML parsers silently accept but GitHub Actions rejects on the default
branch.
Release Note
NONE
Co-authored-by: Claude <claude@anthropic.com>
* ci: address review feedback on yamllint workflow
- Add permissions: contents: read (least privilege)
- Bump checkout to v6.0.2 with persist-credentials: false
- Remove pip install step (yamllint is pre-installed on ubuntu-22.04)
Co-authored-by: Claude <claude@anthropic.com>
---------
Co-authored-by: Claude <claude@anthropic.com>