Skip to content

CI: keep ShakaPerf release workflows YAML-lint clean#4741

Merged
justin808 merged 2 commits into
mainfrom
jg-codex/fix-release-yaml-lint
Jul 18, 2026
Merged

CI: keep ShakaPerf release workflows YAML-lint clean#4741
justin808 merged 2 commits into
mainfrom
jg-codex/fix-release-yaml-lint

Conversation

@justin808

@justin808 justin808 commented Jul 18, 2026

Copy link
Copy Markdown
Member

Why

Independent post-merge QA for release-safety batch ror-release-20260713 found that the two ShakaPerf workflows introduced by the batch did not pass the repository-prescribed YAML lint command. That left the batch blocked despite its behavioral release-helper replay being green.

What changed

  • add explicit YAML document starts and quote the GitHub Actions on key
  • fold long workflow scalars without changing their parsed values
  • wrap embedded Ruby and Bash statements while preserving their commands, environment, outputs, and release-gate behavior

This is a formatting-only, behavior-preserving GitHub Actions change. It does not change triggers, permissions, jobs, conditions, secrets, action versions, commands, or release semantics. Under the repository's post-merge exercise policy, no exercise follow-up issue is required for this non-semantic workflow formatting repair.

Verification

  • python3 -m yamllint -f parsable .github/workflows/shakaperf-release-gates.yml .github/workflows/shakaperf-release-prerun.yml
  • actionlint
  • embedded workflow Bash: bash -n
  • embedded workflow Ruby: ruby -c
  • ShakaPerf workflow contract specs: 3 examples, 0 failures
  • full release-helper spec in independent review: 734 examples, 0 failures
  • CI-equivalent RuboCop: 237 files, no offenses
  • script/ci-changes-detector origin/main
  • git diff --check origin/main...HEAD
  • exact gpt-5.6-sol/xhigh pre-push review: clean, no actionable findings

The repository-wide yamllint .github/ command still reports unrelated pre-existing violations; both changed workflow files pass the exact targeted QA command.

QA finding disposition

  • QA-1 (targeted YAML lint failure): fixed at 84eba0187ff448218600655ad3109d9397c58611
  • Process-gap mechanism target: script
  • Motivating miss: changed release workflows reached post-merge QA without satisfying the documented YAML lint command
  • Replay evidence: the original two-file command failed with 39 errors and 4 warnings at batch merge head 49835c72b00a5ae778f64b841225daa99f887316 and passes at this PR head
  • Non-goal: broad formatting cleanup of unrelated historical workflow files

Summary by CodeRabbit

  • Chores
    • Improved readability across release validation and performance testing workflows by converting long YAML and command lines into folded/multi-line formats.
    • Reworked generated step summary output to use clearer multi-line formatting while keeping the same conditions and content.
    • Enhanced timeout error formatting and log tailing for readiness checks.
    • Kept gate behavior and evidence/compare logic the same, including existing probe and exit handling, while simplifying argument construction and shell assignments.

@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 76bd3687-8a21-4128-8953-6e0d1bd7e20f

📥 Commits

Reviewing files that changed from the base of the PR and between 84eba01 and 000930b.

📒 Files selected for processing (1)
  • .github/workflows/shakaperf-release-gates.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/shakaperf-release-gates.yml

Walkthrough

The ShakaPerf release gates and prerun workflows are reformatted with multiline YAML, Ruby, and shell expressions. Existing validation, readiness probes, comparison behavior, shutdown handling, and evidence output remain unchanged.

Changes

ShakaPerf workflow updates

Layer / File(s) Summary
Workflow metadata and validation output
.github/workflows/shakaperf-release-gates.yml, .github/workflows/shakaperf-release-prerun.yml
Workflow headers, candidate fingerprint output, validation messages, and step-summary writes are reformatted without changing values or control flow.
Dummy app startup and readiness checks
.github/workflows/shakaperf-release-gates.yml
License values, startup commands, readiness probes, and diagnostic logging use multiline formatting while preserving existing probe and failure behavior.
Comparison, evidence, and shutdown handling
.github/workflows/shakaperf-release-gates.yml
Timeout diagnostics, filter-path construction, PID shutdown handling, and evidence JSON writing are reformatted while preserving their outputs and logic.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: keeping the ShakaPerf release workflows YAML-lint clean.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jg-codex/fix-release-yaml-lint

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@justin808

Copy link
Copy Markdown
Member Author

+ci-status

@github-actions

Copy link
Copy Markdown
Contributor

CI Status

Head SHA: 84eba0187ff4
Changed files: 2
Docs-only heuristic (matches ci-changes-detector metadata paths): no
ready-for-hosted-ci label: absent
force-full-hosted-ci label: absent
Current hosted-CI waiver: not present for this SHA
Automatic release-target hosted mode: inactive
Observed exact-head coverage: modes[missing=9]; successful=0, pending=0, failed=0, missing=9

Only the required gate is active unless hosted CI is requested.

@justin808

Copy link
Copy Markdown
Member Author

+ci-force-full

@github-actions

github-actions Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Force-Full Hosted CI Requested

Triggered 9 workflow(s) for 84eba0187ff4.
Skipped 0 workflow(s) with equivalent exact-head coverage.
Mode: force-full hosted CI (bypasses optimized change selection).
Added ready-for-hosted-ci and force-full-hosted-ci, so future commits will bypass optimized hosted CI selection until +ci-stop-full is used.

View progress in the Actions tab.

@greptile-apps

greptile-apps Bot commented Jul 18, 2026

Copy link
Copy Markdown

Greptile Summary

This PR reformats the ShakaPerf release workflows for YAML lint compliance. The main changes are:

  • Adds explicit YAML document starts and quotes the on keys.
  • Folds long workflow names, expressions, and environment values.
  • Wraps embedded Bash and Ruby without changing release-gate behavior.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
.github/workflows/shakaperf-release-gates.yml Reformats YAML scalars, shell commands, and embedded Ruby while preserving candidate validation, service handling, gate execution, and evidence generation.
.github/workflows/shakaperf-release-prerun.yml Reformats the trigger key and embedded Ruby while preserving candidate detection, summary output, and workflow dispatch.

Reviews (1): Last reviewed commit: "Keep ShakaPerf release workflows lintabl..." | Re-trigger Greptile

@github-actions github-actions Bot added force-full-hosted-ci Bypass optimized hosted CI selection and run all hosted suites ready-for-hosted-ci Run optimized hosted GitHub CI for this PR labels Jul 18, 2026
Comment thread .github/workflows/shakaperf-release-gates.yml Outdated
@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Review Summary

What this PR does: Reformats two ShakaPerf release-gate GitHub Actions workflows (shakaperf-release-gates.yml, shakaperf-release-prerun.yml) to satisfy the repo's yamllint command — adding --- document starts, quoting the on: key, folding long scalars (run-name, job name, secret-backed env vars), and wrapping long embedded Ruby/Bash statements. No triggers, permissions, jobs, conditions, action versions, or command semantics are changed.

Verification of "behavior-preserving" claim

I traced each reformatted Ruby string-concatenation and Bash line-continuation against the original to confirm the parsed/executed value is unchanged:

  • All Ruby adjacent-string-literal splits (e.g. the abort "..." and summary.puts "..." calls) reconstruct to byte-identical strings, including whitespace at the split points.
  • All Bash \-continued commands (multi-line curl, pnpm, rails s, echo "::error::..." with implicit space-joining, for pidfile in ...) reconstruct to equivalent commands/output.
  • The YAML folded scalars (>-) for run-name, job name: fields, and RUNTIME_TREE_FINGERPRINT correctly fold to the original single-line value.
  • Quoting on:"on" and adding --- are no-ops for the GitHub Actions parser.

This is careful, well-executed mechanical work overall.

Issue found

One spot is a behavior change despite the PR's "formatting-only" claim — left as an inline comment on shakaperf-release-gates.yml line 124-125: folding SECRET_KEY_BASE: dummy-secret-key-base-for-shakaperf-rsc-fouc # gitleaks:allow into a block scalar separates the # gitleaks:allow suppression comment from the actual secret-value line. gitleaks' inline-allowlist only applies to findings on the same physical line as the comment, so this either silently drops the intended suppression or stops the line from matching the original detection rule at all — a real change in scanning behavior, not just formatting.

Minor style note

In the "Run RSC FOUC ShakaPerf gate" step, the --filter argument was shortened via a new filter_path shell variable (concatenated from two literals) rather than the \-continuation style used everywhere else in the diff for long lines. Not a bug, but it's an inconsistent technique for the same problem (line length) solved elsewhere in this PR — a plain multi-line backslash continuation would match the rest of the file's style.

Other checks

  • No test coverage changes — appropriate, since this is workflow-YAML-only formatting with no logic change.
  • No secrets, permissions, or trigger surface changes — confirmed by reading the diff line by line.

@coderabbitai coderabbitai Bot left a comment

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.

🧹 Nitpick comments (2)
.github/workflows/shakaperf-release-prerun.yml (1)

1-8: 📐 Maintainability & Code Quality | 🔵 Trivial

Workflow Change Audit

As per coding guidelines, here is the audit of the workflow changes:

  • Secrets: Relies on the standard github.token without any changes.
  • Permissions: actions: write and contents: read permissions are maintained. The write permission is required for dispatching workflows.
  • Triggers: The on trigger was quoted as "on": to pass yamllint, with trigger events unchanged.
  • Third-party actions: Action versions remain unchanged.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/shakaperf-release-prerun.yml around lines 1 - 8, Keep the
workflow metadata unchanged: retain the quoted "on" key for yamllint
compatibility, preserve actions: write and contents: read permissions, and do
not alter the existing triggers or third-party action versions.

Source: Coding guidelines

.github/workflows/shakaperf-release-gates.yml (1)

1-10: 📐 Maintainability & Code Quality | 🔵 Trivial

Workflow Change Audit

As per coding guidelines, here is the audit of the workflow changes:

  • Secrets: No new secrets introduced. The existing REACT_ON_RAILS_PRO_LICENSE_V2 secret usage was strictly preserved.
  • Permissions: Safe contents: read permissions are maintained.
  • Triggers: The on trigger was quoted as "on": to pass yamllint truthy value checks, with trigger events unchanged.
  • Third-party actions: Action versions remain unchanged and stable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/shakaperf-release-gates.yml around lines 1 - 10, No
implementation changes are required; preserve the existing workflow
configuration, including the quoted "on" key, contents: read permissions,
unchanged triggers, existing secret usage, and current third-party action
versions.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In @.github/workflows/shakaperf-release-gates.yml:
- Around line 1-10: No implementation changes are required; preserve the
existing workflow configuration, including the quoted "on" key, contents: read
permissions, unchanged triggers, existing secret usage, and current third-party
action versions.

In @.github/workflows/shakaperf-release-prerun.yml:
- Around line 1-8: Keep the workflow metadata unchanged: retain the quoted "on"
key for yamllint compatibility, preserve actions: write and contents: read
permissions, and do not alter the existing triggers or third-party action
versions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 86d7f728-de34-48bc-ab28-801329c93e94

📥 Commits

Reviewing files that changed from the base of the PR and between 18bdc4f and 84eba01.

📒 Files selected for processing (2)
  • .github/workflows/shakaperf-release-gates.yml
  • .github/workflows/shakaperf-release-prerun.yml

@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Review

Reviewed both changed files (shakaperf-release-gates.yml, shakaperf-release-prerun.yml) line-by-line against the pre-change versions to confirm the formatting changes are behavior-preserving. Summary below.

Overview

Formatting-only fix to make the two ShakaPerf release-gate workflows pass yamllint: adds --- document starts, quotes the on: key, folds long GH Actions scalars (run-name, job name, env values) with >-, and wraps long embedded Ruby/Bash lines.

Correctness — verified, no behavior changes found

I manually reconstructed the concatenated strings/commands to check for regressions from the line-wrapping, since that's the highest-risk part of a change like this:

  • Ruby string splits (e.g. the abort "..." and summary = if candidate ... end blocks in both files, plus the run_url construction in the evidence step): re-concatenating the split literals reproduces the original strings exactly — no missing/extra spaces at the split points.
  • echo argument splitting (Check Pro license secret step, and the Timed out waiting for renderer... error message): these switch from a single quoted string to multiple quoted args passed to echo. Since echo joins multiple args with a single space, and the split points were chosen where the original string already had exactly one space, the printed output is identical to before.
  • Bash line continuations (RENDERER_PORT=..., PORT=... REACT_RENDERER_URL=..., the curl readiness checks, tail, the for pidfile in ... loop): straightforward backslash-continuation, no semantic change — env-var-prefixed commands and &&-chained conditionals both continue correctly across the added line breaks.
  • YAML folded scalars (run-name, job names, REACT_ON_RAILS_PRO_LICENSE, RUNTIME_TREE_FINGERPRINT, runtime_tree_fingerprint output): >- folds line breaks to a single space, so the parsed values match the original single-line values.
  • "on": quoting: cosmetic only — GitHub Actions treats on:/"on": identically; this just satisfies YAML's truthy lint rule.
  • The standalone # yamllint disable-line rule:line-length comment placed above the SECRET_KEY_BASE line (gates workflow, line 124) is valid yamllint syntax — a disable-line comment on its own line applies to the following line, not itself, so this correctly suppresses the length warning for the gitleaks:allow line without needing to break that line (which would risk the gitleaks allowlist regex).

Minor observations (non-blocking)

  • In Run RSC FOUC ShakaPerf gate, the --filter value was extracted into a filter_path shell variable built via two-line concatenation (filter_path="..." / filter_path+="...") rather than using backslash line-continuation like the rest of the file. It's correct (the concatenated value matches the original path exactly), but it's a slightly different technique from the "fold scalars"/"wrap statements" pattern used everywhere else — worth a passing thought on consistency, not a real issue.
  • No test coverage changes are needed/expected here since this is workflow-YAML-only; the PR body's cited verification (yamllint, actionlint, bash -n, ruby -c, the ShakaPerf contract specs) is the appropriate check for this kind of change.

Security

No security-relevant changes — permissions, secrets usage (REACT_ON_RAILS_PRO_LICENSE_V2), and the gitleaks:allow dummy secret are all preserved as-is, just reformatted/folded.

Overall: LGTM. This is a careful, low-risk, behavior-preserving formatting fix — I did not find any actual bugs introduced by the reformatting.

@justin808

Copy link
Copy Markdown
Member Author

Address-review summary

Scan scope: full PR review history through 2026-07-18T10:24:49Z.

Mattered

  • MUST-FIX: Claude’s gitleaks:allow line-scoping finding was independently confirmed and fixed in 000930bb4747d693866ad117c99ae8260dd54c1d; the review thread was replied to with exact validation evidence and resolved.

Skipped

  • CodeRabbit’s two workflow-audit notes explicitly requested no implementation change; no action was required because triggers, permissions, secret references, and action versions remain unchanged.

No deferred work or follow-up issue was created. This pass used an explicit full-history scan after the current-head security preflight.

Next default scan starts after this comment. Say check all reviews to rescan the full PR.

@justin808
justin808 added this pull request to the merge queue Jul 18, 2026
Merged via the queue into main with commit adf0c47 Jul 18, 2026
50 checks passed
@justin808
justin808 deleted the jg-codex/fix-release-yaml-lint branch July 18, 2026 11:07
justin808 added a commit that referenced this pull request Jul 18, 2026
…t-policy

* origin/main:
  CI: keep ShakaPerf release workflows YAML-lint clean (#4741)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

force-full-hosted-ci Bypass optimized hosted CI selection and run all hosted suites ready-for-hosted-ci Run optimized hosted GitHub CI for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant