Skip to content

fix(ci): preserve stable ancestry in main sync#3817

Closed
caio-pizzol wants to merge 9 commits into
mainfrom
fix/direct-stable-main-sync
Closed

fix(ci): preserve stable ancestry in main sync#3817
caio-pizzol wants to merge 9 commits into
mainfrom
fix/direct-stable-main-sync

Conversation

@caio-pizzol

Copy link
Copy Markdown
Contributor

Preserves stable release ancestry when syncing stable back into main, so semantic-release can calculate @next versions from the latest stable release instead of continuing an older prerelease line.

  • pushes the already-verified merge commit directly to main
  • keeps the existing version-only conflict handling and ancestry check
  • removes the squashable sync PR and its unused permissions
  • uses a normal push that fails if main advances

@caio-pizzol
caio-pizzol marked this pull request as ready for review July 13, 2026 19:56
@caio-pizzol
caio-pizzol requested a review from a team as a code owner July 13, 2026 19:56
@qodo-code-review

qodo-code-review Bot commented Jul 13, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Wrong run may be checked ✓ Resolved 🐞 Bug ≡ Correctness
Description
require_successful_release checks only the latest stable run (--limit 1) and fails immediately
if that run is in-progress or not success, without correlating it to the stable revision being
synced. This can make the sync flaky (new run starts after the drain step) or validate the wrong run
when multiple stable runs exist.
Code

.github/workflows/sync-patches.yml[R102-115]

+            run=$(gh run list \
+              --repo "$REPO" \
+              --workflow "$workflow_file" \
+              --branch stable \
+              --limit 1 \
+              --json conclusion,status,url)
+            status=$(jq -r '.[0].status // "missing"' <<< "$run")
+            conclusion=$(jq -r '.[0].conclusion // "missing"' <<< "$run")
+            url=$(jq -r '.[0].url // "unavailable"' <<< "$run")
+
+            if [ "$status" != "completed" ] || [ "$conclusion" != "success" ]; then
+              echo "$label release is not successful: status=$status conclusion=$conclusion url=$url"
+              exit 1
+            fi
Relevance

⭐⭐⭐ High

Team previously accepted “moving ref validates wrong revision” release-automation bug; see accepted
fix in PR #2989.

PR-#2989
PR-#3331

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The verification function explicitly queries only one run on stable and fails if that single run
is not completed/success. It also computes required checks from diffs against moving refs
(origin/main...origin/stable) after fetching, so the verification is not tied to a stable
snapshot; this is the same moving-ref class of issue previously accepted as a bug pattern in PR
#2989.

.github/workflows/sync-patches.yml[84-116]
.github/workflows/sync-patches.yml[92-107]
.github/workflows/sync-patches.yml[120-138]
.github/workflows/sync-patches.yml[35-70]
PR-#2989

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new `Verify stable release lane succeeded` step selects only the most recent workflow run on `stable` (`--limit 1`) and treats anything other than `completed/success` as failure. Because it does not correlate the selected run(s) to a pinned snapshot of what will be merged (and does not re-wait on new runs), it can either:
- fail spuriously if a new stable run starts between the drain and verify steps, or
- accept an unrelated successful run.

## Issue Context
This workflow is meant to ensure `main` only advances from a stable head whose relevant release workflows succeeded.

## Fix Focus Areas
- .github/workflows/sync-patches.yml[84-139]

## Suggested fix approach
1. Capture a stable snapshot early in the verify step (e.g., `MAIN_SHA=$(git rev-parse origin/main)` and `STABLE_SHA=$(git rev-parse origin/stable)` after the fetch).
2. When querying workflow runs, select the most recent **completed** run and ensure it is consistent with the snapshot (examples):
  - Prefer filtering to `status==completed` (don’t fail just because the newest run is in progress).
  - Additionally, ensure no newer runs started after your snapshot time, or re-run the drain check after verification.
3. Consider passing these snapshot SHAs to the later merge/push step so the merge uses the same `STABLE_SHA` you verified (instead of re-fetching and using a potentially moved `origin/stable`).
4. Update the unit test accordingly (it currently enshrines `--limit 1`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Unchecked slice bounds ✓ Resolved 🐞 Bug ☼ Reliability
Description
The test slices the workflow text using indexOf() results without asserting the step markers were
found, so a renamed/moved step can make the slice bounds -1 and the later assertion can pass while
inspecting the wrong substring. This weakens the test’s ability to catch regressions in the
lane-drain step gating.
Code

scripts/tests/release-local.test.mjs[R393-396]

+  const laneDrainStep = workflow.slice(
+    workflow.indexOf('- name: Wait for stable release lane to drain'),
+    workflow.indexOf('- name: Generate token'),
+  );
Relevance

⭐⭐⭐ High

Team often hardens tests to prevent false positives/brittleness (e.g., fix always-pass assertions,
add explicit guards).

PR-#2832
PR-#2318
PR-#3104

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test computes laneDrainStep directly from indexOf() results and later relies on it to assert
the absence of an if: guard; without checking for -1, slice() can select the wrong region
while the negative includes() assertion still returns false. The workflow currently contains
both step names, so this won’t fail today, but it will become brittle/unsafe on future refactors.

scripts/tests/release-local.test.mjs[391-442]
.github/workflows/sync-patches.yml[31-71]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`laneDrainStep` is derived via `workflow.slice(workflow.indexOf(...), workflow.indexOf(...))` without validating that both markers exist and are ordered. Because `String.prototype.slice()` accepts negative indices, missing markers can yield an empty/unrelated substring and make the subsequent `laneDrainStep.includes(...)` assertion meaningless.

### Issue Context
This is a test-only issue but it can produce false confidence (tests stay green while the workflow structure changes).

### Fix Focus Areas
- scripts/__tests__/release-local.test.mjs[391-442]

### Suggested fix
1. Store the indices in variables: `start`, `end`.
2. Add explicit assertions:
  - `assert.notEqual(start, -1, '...missing Wait for stable...')`
  - `assert.notEqual(end, -1, '...missing Generate token...')`
  - `assert.ok(start < end, '...expected Wait... before Generate token...')`
3. Slice only after these checks.

(Alternative: use a regex to capture the step block instead of positional slicing.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Skip-ci gate too broad ✓ Resolved 🐞 Bug ≡ Correctness
Description
In sync-patches.yml, the pinned-stable verification treats any intervening commit whose subject ends
with "[skip ci]" as verified, so a non-release change that happens to end with that token will be
merged into main without being flagged. This defeats the intended “only release writebacks after the
successful trigger” safety check and can allow unexpected stable commits into main.
Code

.github/workflows/sync-patches.yml[R101-111]

+            unexpected_commit=false
+            while read -r sha subject; do
+              [ -z "$sha" ] && continue
+              case "$subject" in
+                *"[skip ci]") ;;
+                *)
+                  echo "Pinned stable includes a newer unverified commit: $sha $subject"
+                  unexpected_commit=true
+                  ;;
+              esac
+            done < <(git log --format='%H %s' "$TRIGGER_STABLE_SHA..$stable_sha")
Relevance

⭐⭐ Medium

No direct precedent on narrowing [skip ci] subject whitelist; team accepts some workflow hardening,
rejects others.

PR-#2989
PR-#2938
PR-#2644

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow’s new verification explicitly whitelists commits solely based on a [skip ci] suffix
in the subject, while the repo’s own tooling defines release automation commits more strictly (and
case-insensitively) as chore(...) ... [skip ci]. This mismatch shows the workflow gate can accept
non-release commits that merely end with [skip ci].

.github/workflows/sync-patches.yml[95-115]
scripts/semantic-release/linear-commit-sync.cjs[71-74]
packages/superdoc/.releaserc.cjs[97-106]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The stable→main sync verification currently accepts any commit between `TRIGGER_STABLE_SHA` and the pinned `stable_sha` if its *subject* matches the glob `*"[skip ci]"`. That check is too permissive because it does not validate that the commit is actually a semantic-release writeback.

## Issue Context
The repo already has a stricter notion of “release automation commit” (`isReleaseAutomationCommit`) that requires a conventional `chore(...)` subject with a `[skip ci]` suffix, case-insensitive.

## Fix Focus Areas
- .github/workflows/sync-patches.yml[95-116]

## Suggested fix approach
- Replace the `case "$subject" in *"[skip ci]")` allowlist with a stricter predicate that matches the repo’s release writeback format, e.g.:
 - allow only subjects matching `^chore(\([^)]+\))?: .*\[skip ci\]\s*$` (case-insensitive)
 - optionally also require author/committer to be the automation bot (e.g., `github-actions[bot]`) to prevent manual `[skip ci]` commits from bypassing the gate.
- Implement case-insensitive matching in bash via `shopt -s nocasematch` + `[[ ... =~ regex ]]`, or via `grep -Eqi`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Dispatch failures ignored ✓ Resolved 🐞 Bug ☼ Reliability
Description
The stable-lane failure check builds release_runs via gh run list --event push, so failed stable
release workflows started via workflow_dispatch are excluded and won’t block the automatic
stable→main sync. This can contradict the workflow’s intent to require the whole stable release lane
to succeed before advancing main.
Code

.github/workflows/sync-patches.yml[R74-89]

+          release_runs=$(gh run list \
+            --repo "$REPO" \
+            --branch stable \
+            --event push \
+            --limit 100 \
+            --json conclusion,createdAt,headSha,name,status,url)
+
+          failed_runs=$(jq \
+            --arg head_sha "$RELEASE_HEAD_SHA" \
+            --arg created_at "$RELEASE_CREATED_AT" \
+            '[.[] | select(
+              (.name == "📦 Release stable tooling (CLI/SDK/MCP)" or .name == "📦 Release esign" or .name == "📦 Release template-builder") and
+              .status == "completed" and
+              (.headSha == $head_sha or .createdAt >= $created_at) and
+              (.conclusion == "failure" or .conclusion == "cancelled" or .conclusion == "timed_out" or .conclusion == "action_required" or .conclusion == "startup_failure" or .conclusion == "stale")
+            )]' <<< "$release_runs")
Relevance

⭐⭐ Medium

No precedent on gh run list --event push excluding dispatch; dispatch used for recovery in similar
workflows.

PR-#3080
PR-#3204
PR-#3331

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
release_runs is sourced only from push-triggered runs, but the stable release workflows being
gated explicitly allow manual dispatch; therefore a failed manually-dispatched stable release run
won’t appear in release_runs and cannot be flagged by the jq failure filter.

.github/workflows/sync-patches.yml[3-7]
.github/workflows/sync-patches.yml[74-89]
.github/workflows/release-stable.yml[13-18]
.github/workflows/release-esign.yml[4-14]
.github/workflows/release-template-builder.yml[4-14]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The sync workflow’s failed-run gate only inspects workflow runs triggered by `push` (`gh run list --event push`). Stable release workflows also support `workflow_dispatch`, so failures from manually dispatched stable release runs won’t be detected and can allow the auto-sync to proceed.

### Issue Context
The workflow comment says the automatic sync should wait for stable release workflows to succeed before syncing stable into main.

### Fix Focus Areas
- .github/workflows/sync-patches.yml[74-89]

### Suggested fix
Update the `release_runs` collection to include `workflow_dispatch` (and possibly omit `--event` entirely) so the subsequent `jq` filter can detect failures regardless of how the run was started.
Examples:
- Remove `--event push` and rely on the existing `.headSha == $head_sha or .createdAt >= $created_at` scoping.
- Or query both events and merge JSON arrays (push + workflow_dispatch) before filtering.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

5. Test locks in brittle strings ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The new test asserts exact implementation strings like --limit 1 and the exact `git diff
--name-only origin/main...origin/stable` invocation, so semantically-equivalent fixes (e.g.,
selecting completed runs, pinning to a snapshot SHA) will break tests. This will make improving the
verification logic harder and encourages keeping the current fragile selection behavior.
Code

scripts/tests/release-local.test.mjs[R435-440]

+    workflow.includes('Verify stable release lane succeeded') &&
+      workflow.includes('--workflow "$workflow_file"') &&
+      workflow.includes('--limit 1') &&
+      workflow.includes('git diff --name-only origin/main...origin/stable') &&
+      workflow.includes('require_successful_release release-esign.yml') &&
+      workflow.includes('require_successful_release release-template-builder.yml'),
Relevance

⭐ Low

Repo frequently uses string/regex pinning in release tests as regression guards (e.g., PRs #3337,
#3348).

PR-#3337
PR-#3348

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test explicitly requires the workflow to contain --limit 1 and other exact command fragments,
tightly coupling it to the current implementation rather than the intended contract.

scripts/tests/release-local.test.mjs[424-442]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test over-specifies workflow implementation details (specific CLI flags/strings), making legitimate refactors and correctness fixes fail the test.

## Issue Context
The workflow verification logic likely needs to evolve (e.g., avoid `--limit 1` races, filter to completed runs, or pin to a snapshot). The current assertions would block those changes.

## Fix Focus Areas
- scripts/__tests__/release-local.test.mjs[435-441]

## Suggested fix approach
- Replace checks for exact substrings like `--limit 1` / exact `git diff ...origin/main...origin/stable` with assertions for higher-level intent, e.g.:
 - The workflow has a `Verify stable release lane succeeded` step.
 - It queries stable workflow runs via `gh run list` for the expected workflows.
 - It enforces `status==completed` and `conclusion==success` semantics (string checks for those comparisons are OK), without enforcing exact query size/flags.
- If you move to snapshot/SHA-aware logic, assert for the presence of that snapshot variable usage instead of fixed flags.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Direct push skips PR CI 🐞 Bug ☼ Reliability
Description
The sync workflow now updates main via git push origin HEAD:main instead of opening a PR. Checks
that run only on pull_request/merge_group (and not on push) will not execute for this update,
so if branch protection allows the push, main can advance without those validations.
Code

.github/workflows/sync-patches.yml[R174-177]

+          # The merge commit must land unchanged so semantic-release can see
+          # stable release tags in main's ancestry. A normal push fails safely
+          # if main advanced while this job was running.
+          git push origin HEAD:main
Relevance

⭐ Low

Team treats many CI workflows as PR-only/optional; direct branch pushes in workflows accepted (PR
#3036, #2989).

PR-#3036
PR-#2989

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow now pushes directly to main, while multiple CI workflows are configured to run only
on PR/merge-queue events, meaning they will not run for a direct push event to main.

.github/workflows/sync-patches.yml[174-177]
.github/workflows/ci-superdoc.yml[6-11]
.github/workflows/ci-react.yml[6-11]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`sync-patches.yml` now pushes directly to `main`, which bypasses workflows that only run on `pull_request`/`merge_group`. If branch protection permits the push, this can advance `main` without the usual CI validation.

## Issue Context
This repo has important CI workflows (e.g., `CI SuperDoc`) that do not run on `push` to `main`, so the direct push path is materially different from the prior PR-based sync.

## Fix Focus Areas
- .github/workflows/sync-patches.yml[174-177]

## Suggested fix options
Choose one (or combine):
1) Re-introduce a PR-based sync (still using the already-created merge commit), so PR-only CI runs before landing on `main`.
2) Add an explicit validation phase in this workflow (or call a reusable workflow via `workflow_call`) that runs the same checks you rely on for `main` before executing `git push origin HEAD:main`.
3) Alternatively, update the critical CI workflows to also trigger on `push` to `main` (where appropriate), so the direct push is still covered.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit f150f29

Results up to commit 9b1fe2f


🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Informational
1. Direct push skips PR CI 🐞 Bug ☼ Reliability
Description
The sync workflow now updates main via git push origin HEAD:main instead of opening a PR. Checks
that run only on pull_request/merge_group (and not on push) will not execute for this update,
so if branch protection allows the push, main can advance without those validations.
Code

.github/workflows/sync-patches.yml[R174-177]

+          # The merge commit must land unchanged so semantic-release can see
+          # stable release tags in main's ancestry. A normal push fails safely
+          # if main advanced while this job was running.
+          git push origin HEAD:main
Relevance

⭐ Low

Team treats many CI workflows as PR-only/optional; direct branch pushes in workflows accepted (PR
#3036, #2989).

PR-#3036
PR-#2989

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow now pushes directly to main, while multiple CI workflows are configured to run only
on PR/merge-queue events, meaning they will not run for a direct push event to main.

.github/workflows/sync-patches.yml[174-177]
.github/workflows/ci-superdoc.yml[6-11]
.github/workflows/ci-react.yml[6-11]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`sync-patches.yml` now pushes directly to `main`, which bypasses workflows that only run on `pull_request`/`merge_group`. If branch protection permits the push, this can advance `main` without the usual CI validation.

## Issue Context
This repo has important CI workflows (e.g., `CI SuperDoc`) that do not run on `push` to `main`, so the direct push path is materially different from the prior PR-based sync.

## Fix Focus Areas
- .github/workflows/sync-patches.yml[174-177]

## Suggested fix options
Choose one (or combine):
1) Re-introduce a PR-based sync (still using the already-created merge commit), so PR-only CI runs before landing on `main`.
2) Add an explicit validation phase in this workflow (or call a reusable workflow via `workflow_call`) that runs the same checks you rely on for `main` before executing `git push origin HEAD:main`.
3) Alternatively, update the critical CI workflows to also trigger on `push` to `main` (where appropriate), so the direct push is still covered.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit 5d2b3b9


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Remediation recommended
1. Dispatch failures ignored ✓ Resolved 🐞 Bug ☼ Reliability
Description
The stable-lane failure check builds release_runs via gh run list --event push, so failed stable
release workflows started via workflow_dispatch are excluded and won’t block the automatic
stable→main sync. This can contradict the workflow’s intent to require the whole stable release lane
to succeed before advancing main.
Code

.github/workflows/sync-patches.yml[R74-89]

+          release_runs=$(gh run list \
+            --repo "$REPO" \
+            --branch stable \
+            --event push \
+            --limit 100 \
+            --json conclusion,createdAt,headSha,name,status,url)
+
+          failed_runs=$(jq \
+            --arg head_sha "$RELEASE_HEAD_SHA" \
+            --arg created_at "$RELEASE_CREATED_AT" \
+            '[.[] | select(
+              (.name == "📦 Release stable tooling (CLI/SDK/MCP)" or .name == "📦 Release esign" or .name == "📦 Release template-builder") and
+              .status == "completed" and
+              (.headSha == $head_sha or .createdAt >= $created_at) and
+              (.conclusion == "failure" or .conclusion == "cancelled" or .conclusion == "timed_out" or .conclusion == "action_required" or .conclusion == "startup_failure" or .conclusion == "stale")
+            )]' <<< "$release_runs")
Relevance

⭐⭐ Medium

No precedent on gh run list --event push excluding dispatch; dispatch used for recovery in similar
workflows.

PR-#3080
PR-#3204
PR-#3331

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
release_runs is sourced only from push-triggered runs, but the stable release workflows being
gated explicitly allow manual dispatch; therefore a failed manually-dispatched stable release run
won’t appear in release_runs and cannot be flagged by the jq failure filter.

.github/workflows/sync-patches.yml[3-7]
.github/workflows/sync-patches.yml[74-89]
.github/workflows/release-stable.yml[13-18]
.github/workflows/release-esign.yml[4-14]
.github/workflows/release-template-builder.yml[4-14]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The sync workflow’s failed-run gate only inspects workflow runs triggered by `push` (`gh run list --event push`). Stable release workflows also support `workflow_dispatch`, so failures from manually dispatched stable release runs won’t be detected and can allow the auto-sync to proceed.

### Issue Context
The workflow comment says the automatic sync should wait for stable release workflows to succeed before syncing stable into main.

### Fix Focus Areas
- .github/workflows/sync-patches.yml[74-89]

### Suggested fix
Update the `release_runs` collection to include `workflow_dispatch` (and possibly omit `--event` entirely) so the subsequent `jq` filter can detect failures regardless of how the run was started.
Examples:
- Remove `--event push` and rely on the existing `.headSha == $head_sha or .createdAt >= $created_at` scoping.
- Or query both events and merge JSON arrays (push + workflow_dispatch) before filtering.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit 9af2934


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Action required
1. Wrong run may be checked ✓ Resolved 🐞 Bug ≡ Correctness
Description
require_successful_release checks only the latest stable run (--limit 1) and fails immediately
if that run is in-progress or not success, without correlating it to the stable revision being
synced. This can make the sync flaky (new run starts after the drain step) or validate the wrong run
when multiple stable runs exist.
Code

.github/workflows/sync-patches.yml[R102-115]

+            run=$(gh run list \
+              --repo "$REPO" \
+              --workflow "$workflow_file" \
+              --branch stable \
+              --limit 1 \
+              --json conclusion,status,url)
+            status=$(jq -r '.[0].status // "missing"' <<< "$run")
+            conclusion=$(jq -r '.[0].conclusion // "missing"' <<< "$run")
+            url=$(jq -r '.[0].url // "unavailable"' <<< "$run")
+
+            if [ "$status" != "completed" ] || [ "$conclusion" != "success" ]; then
+              echo "$label release is not successful: status=$status conclusion=$conclusion url=$url"
+              exit 1
+            fi
Relevance

⭐⭐⭐ High

Team previously accepted “moving ref validates wrong revision” release-automation bug; see accepted
fix in PR #2989.

PR-#2989
PR-#3331

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The verification function explicitly queries only one run on stable and fails if that single run
is not completed/success. It also computes required checks from diffs against moving refs
(origin/main...origin/stable) after fetching, so the verification is not tied to a stable
snapshot; this is the same moving-ref class of issue previously accepted as a bug pattern in PR
#2989.

.github/workflows/sync-patches.yml[84-116]
.github/workflows/sync-patches.yml[92-107]
.github/workflows/sync-patches.yml[120-138]
.github/workflows/sync-patches.yml[35-70]
PR-#2989

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new `Verify stable release lane succeeded` step selects only the most recent workflow run on `stable` (`--limit 1`) and treats anything other than `completed/success` as failure. Because it does not correlate the selected run(s) to a pinned snapshot of what will be merged (and does not re-wait on new runs), it can either:
- fail spuriously if a new stable run starts between the drain and verify steps, or
- accept an unrelated successful run.

## Issue Context
This workflow is meant to ensure `main` only advances from a stable head whose relevant release workflows succeeded.

## Fix Focus Areas
- .github/workflows/sync-patches.yml[84-139]

## Suggested fix approach
1. Capture a stable snapshot early in the verify step (e.g., `MAIN_SHA=$(git rev-parse origin/main)` and `STABLE_SHA=$(git rev-parse origin/stable)` after the fetch).
2. When querying workflow runs, select the most recent **completed** run and ensure it is consistent with the snapshot (examples):
  - Prefer filtering to `status==completed` (don’t fail just because the newest run is in progress).
  - Additionally, ensure no newer runs started after your snapshot time, or re-run the drain check after verification.
3. Consider passing these snapshot SHAs to the later merge/push step so the merge uses the same `STABLE_SHA` you verified (instead of re-fetching and using a potentially moved `origin/stable`).
4. Update the unit test accordingly (it currently enshrines `--limit 1`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational
2. Test locks in brittle strings ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The new test asserts exact implementation strings like --limit 1 and the exact `git diff
--name-only origin/main...origin/stable` invocation, so semantically-equivalent fixes (e.g.,
selecting completed runs, pinning to a snapshot SHA) will break tests. This will make improving the
verification logic harder and encourages keeping the current fragile selection behavior.
Code

scripts/tests/release-local.test.mjs[R435-440]

+    workflow.includes('Verify stable release lane succeeded') &&
+      workflow.includes('--workflow "$workflow_file"') &&
+      workflow.includes('--limit 1') &&
+      workflow.includes('git diff --name-only origin/main...origin/stable') &&
+      workflow.includes('require_successful_release release-esign.yml') &&
+      workflow.includes('require_successful_release release-template-builder.yml'),
Relevance

⭐ Low

Repo frequently uses string/regex pinning in release tests as regression guards (e.g., PRs #3337,
#3348).

PR-#3337
PR-#3348

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test explicitly requires the workflow to contain --limit 1 and other exact command fragments,
tightly coupling it to the current implementation rather than the intended contract.

scripts/tests/release-local.test.mjs[424-442]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test over-specifies workflow implementation details (specific CLI flags/strings), making legitimate refactors and correctness fixes fail the test.

## Issue Context
The workflow verification logic likely needs to evolve (e.g., avoid `--limit 1` races, filter to completed runs, or pin to a snapshot). The current assertions would block those changes.

## Fix Focus Areas
- scripts/__tests__/release-local.test.mjs[435-441]

## Suggested fix approach
- Replace checks for exact substrings like `--limit 1` / exact `git diff ...origin/main...origin/stable` with assertions for higher-level intent, e.g.:
 - The workflow has a `Verify stable release lane succeeded` step.
 - It queries stable workflow runs via `gh run list` for the expected workflows.
 - It enforces `status==completed` and `conclusion==success` semantics (string checks for those comparisons are OK), without enforcing exact query size/flags.
- If you move to snapshot/SHA-aware logic, assert for the presence of that snapshot variable usage instead of fixed flags.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit f9cde05


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Remediation recommended
1. Unchecked slice bounds ✓ Resolved 🐞 Bug ☼ Reliability
Description
The test slices the workflow text using indexOf() results without asserting the step markers were
found, so a renamed/moved step can make the slice bounds -1 and the later assertion can pass while
inspecting the wrong substring. This weakens the test’s ability to catch regressions in the
lane-drain step gating.
Code

scripts/tests/release-local.test.mjs[R393-396]

+  const laneDrainStep = workflow.slice(
+    workflow.indexOf('- name: Wait for stable release lane to drain'),
+    workflow.indexOf('- name: Generate token'),
+  );
Relevance

⭐⭐⭐ High

Team often hardens tests to prevent false positives/brittleness (e.g., fix always-pass assertions,
add explicit guards).

PR-#2832
PR-#2318
PR-#3104

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test computes laneDrainStep directly from indexOf() results and later relies on it to assert
the absence of an if: guard; without checking for -1, slice() can select the wrong region
while the negative includes() assertion still returns false. The workflow currently contains
both step names, so this won’t fail today, but it will become brittle/unsafe on future refactors.

scripts/tests/release-local.test.mjs[391-442]
.github/workflows/sync-patches.yml[31-71]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`laneDrainStep` is derived via `workflow.slice(workflow.indexOf(...), workflow.indexOf(...))` without validating that both markers exist and are ordered. Because `String.prototype.slice()` accepts negative indices, missing markers can yield an empty/unrelated substring and make the subsequent `laneDrainStep.includes(...)` assertion meaningless.

### Issue Context
This is a test-only issue but it can produce false confidence (tests stay green while the workflow structure changes).

### Fix Focus Areas
- scripts/__tests__/release-local.test.mjs[391-442]

### Suggested fix
1. Store the indices in variables: `start`, `end`.
2. Add explicit assertions:
  - `assert.notEqual(start, -1, '...missing Wait for stable...')`
  - `assert.notEqual(end, -1, '...missing Generate token...')`
  - `assert.ok(start < end, '...expected Wait... before Generate token...')`
3. Slice only after these checks.

(Alternative: use a regex to capture the step block instead of positional slicing.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit 5671e06


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Remediation recommended
1. Skip-ci gate too broad ✓ Resolved 🐞 Bug ≡ Correctness
Description
In sync-patches.yml, the pinned-stable verification treats any intervening commit whose subject ends
with "[skip ci]" as verified, so a non-release change that happens to end with that token will be
merged into main without being flagged. This defeats the intended “only release writebacks after the
successful trigger” safety check and can allow unexpected stable commits into main.
Code

.github/workflows/sync-patches.yml[R101-111]

+            unexpected_commit=false
+            while read -r sha subject; do
+              [ -z "$sha" ] && continue
+              case "$subject" in
+                *"[skip ci]") ;;
+                *)
+                  echo "Pinned stable includes a newer unverified commit: $sha $subject"
+                  unexpected_commit=true
+                  ;;
+              esac
+            done < <(git log --format='%H %s' "$TRIGGER_STABLE_SHA..$stable_sha")
Relevance

⭐⭐ Medium

No direct precedent on narrowing [skip ci] subject whitelist; team accepts some workflow hardening,
rejects others.

PR-#2989
PR-#2938
PR-#2644

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow’s new verification explicitly whitelists commits solely based on a [skip ci] suffix
in the subject, while the repo’s own tooling defines release automation commits more strictly (and
case-insensitively) as chore(...) ... [skip ci]. This mismatch shows the workflow gate can accept
non-release commits that merely end with [skip ci].

.github/workflows/sync-patches.yml[95-115]
scripts/semantic-release/linear-commit-sync.cjs[71-74]
packages/superdoc/.releaserc.cjs[97-106]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The stable→main sync verification currently accepts any commit between `TRIGGER_STABLE_SHA` and the pinned `stable_sha` if its *subject* matches the glob `*"[skip ci]"`. That check is too permissive because it does not validate that the commit is actually a semantic-release writeback.

## Issue Context
The repo already has a stricter notion of “release automation commit” (`isReleaseAutomationCommit`) that requires a conventional `chore(...)` subject with a `[skip ci]` suffix, case-insensitive.

## Fix Focus Areas
- .github/workflows/sync-patches.yml[95-116]

## Suggested fix approach
- Replace the `case "$subject" in *"[skip ci]")` allowlist with a stricter predicate that matches the repo’s release writeback format, e.g.:
 - allow only subjects matching `^chore(\([^)]+\))?: .*\[skip ci\]\s*$` (case-insensitive)
 - optionally also require author/committer to be the automation bot (e.g., `github-actions[bot]`) to prevent manual `[skip ci]` commits from bypassing the gate.
- Implement case-insensitive matching in bash via `shopt -s nocasematch` + `[[ ... =~ regex ]]`, or via `grep -Eqi`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit f6ff703

@caio-pizzol

Copy link
Copy Markdown
Contributor Author

@qodo-code-review Not changing this. The sync carries reviewed stable history plus release versions, and substantive conflicts stop the push.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Comment thread .github/workflows/sync-patches.yml Outdated
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 5d2b3b9

Comment thread .github/workflows/sync-patches.yml Outdated
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 9af2934

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit fc96492

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 6047b95

Comment thread scripts/__tests__/release-local.test.mjs Outdated
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit f9cde05

Comment thread .github/workflows/sync-patches.yml
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 5671e06

@caio-pizzol

Copy link
Copy Markdown
Contributor Author

@qodo-code-review Fixed in f150f29. Release writebacks now require the semantic-release identity, strict release subject, and version-file-only changes.

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit f150f29

@superdoc-bot

superdoc-bot Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

This pull request landed on main as 53fe16d.

@superdoc-bot superdoc-bot Bot closed this in 53fe16d Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants