Skip to content

improvement(ci): fix companion regex#5083

Merged
Sg312 merged 1 commit into
stagingfrom
dev
Jun 16, 2026
Merged

improvement(ci): fix companion regex#5083
Sg312 merged 1 commit into
stagingfrom
dev

Conversation

@Sg312

@Sg312 Sg312 commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fix companion workflow regex

Type of Change

  • Bug fix

Testing

Manual

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Mirror of copilot: move TRAILER/REF into parseCompanions + use matchAll instead
of module-scoped g-flagged regexes with manual lastIndex resets (greptile P2).
Behavior unchanged.
@vercel

vercel Bot commented Jun 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
docs Ready Ready Preview, Comment Jun 16, 2026 2:01am

Request Review

@greptile-apps

greptile-apps Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR refactors the companion PR check GitHub Actions workflow: it removes the scheduled 30-minute cron trigger (relying on PR events and manual workflow_dispatch instead), moves regex literals inside parseCompanions and switches to matchAll() to eliminate shared lastIndex state, renames the label from has-companion to directional per-repo labels (requires-mothership-merge / requires-sim-merge), and improves error handling on label API calls.

  • Regex fix: TRAILER and REF were previously module-level constants with global flags; by moving them into parseCompanions and replacing exec-loop with matchAll, each invocation gets a fresh iterator with no cross-call lastIndex leakage.
  • Label rename: The static has-companion label is replaced by two directional labels (requires-mothership-merge on sim, requires-sim-merge on the other repo), giving clearer merge-order semantics.
  • Cron removal: The periodic 30-minute re-scan is dropped; PRs that open before a companion merges will only refresh when the PR author re-edits or triggers workflow_dispatch manually.

Confidence Score: 4/5

Safe to merge — the regex scoping and matchAll migration are correct, and the label/error-handling changes are sound. The two findings are cosmetic/operational and do not affect the core check logic.

The core regex fix (moving TRAILER/REF inside parseCompanions and switching to matchAll) is correct and eliminates the class of shared-state bugs. Label rename logic is consistent across ensureLabel/addLabels/clear. The only gaps are a leftover exec-loop inconsistency on SQUASH (not a runtime bug, just style) and the fact that the old has-companion label will linger on existing PRs with no automatic cleanup path.

.github/workflows/companion-pr-check.yml — the SQUASH exec loop and the has-companion label migration are worth a second look before merging.

Important Files Changed

Filename Overview
.github/workflows/companion-pr-check.yml Core workflow file; regex scoping fix and matchAll migration are correct, label logic is sound, but SQUASH exec loop is inconsistently left on the old pattern and old has-companion labels on existing PRs won't be cleaned up.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([PR opened / edited / synced\nor workflow_dispatch]) --> B{Event type?}
    B -- pull_request --> C[checkPR on context.payload.pull_request]
    B -- workflow_dispatch --> D[Paginate open PRs on staging & main]
    D --> C

    C --> E[collectCompanions pr]
    E --> F[parseCompanions pr.body\nTRAILER + REF via matchAll]
    F --> G{base = main?}
    G -- yes --> H[Paginate commits\nSQUASH regex → featurePR numbers]
    H --> I[parseCompanions each feature PR body]
    I --> E2[Merge & de-dup companion list]
    G -- no --> E2

    E2 --> J{companions.length === 0?}
    J -- yes --> K[clear: delete sticky comment\nremoveLabel LABEL]
    J -- no --> L[ensureLabel\ngetLabel → 404 → createLabel]
    L --> M[addLabels LABEL to PR]
    M --> N[For each companion\ncrossGetPR via PAT fetch]
    N --> O{companion merged?}
    O -- yes, same base --> P[✅ line]
    O -- yes, wrong base --> Q[⚠️ line\nwarn=true]
    O -- no --> R[❌ line\nwarn=true]
    O -- error --> S[❓ line\nwarn=true]
    P & Q & R & S --> T[upsert sticky comment\nwith heading + lines]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A([PR opened / edited / synced\nor workflow_dispatch]) --> B{Event type?}
    B -- pull_request --> C[checkPR on context.payload.pull_request]
    B -- workflow_dispatch --> D[Paginate open PRs on staging & main]
    D --> C

    C --> E[collectCompanions pr]
    E --> F[parseCompanions pr.body\nTRAILER + REF via matchAll]
    F --> G{base = main?}
    G -- yes --> H[Paginate commits\nSQUASH regex → featurePR numbers]
    H --> I[parseCompanions each feature PR body]
    I --> E2[Merge & de-dup companion list]
    G -- no --> E2

    E2 --> J{companions.length === 0?}
    J -- yes --> K[clear: delete sticky comment\nremoveLabel LABEL]
    J -- no --> L[ensureLabel\ngetLabel → 404 → createLabel]
    L --> M[addLabels LABEL to PR]
    M --> N[For each companion\ncrossGetPR via PAT fetch]
    N --> O{companion merged?}
    O -- yes, same base --> P[✅ line]
    O -- yes, wrong base --> Q[⚠️ line\nwarn=true]
    O -- no --> R[❌ line\nwarn=true]
    O -- error --> S[❓ line\nwarn=true]
    P & Q & R & S --> T[upsert sticky comment\nwith heading + lines]
Loading

Comments Outside Diff (2)

  1. .github/workflows/companion-pr-check.yml, line 139-145 (link)

    P2 The SQUASH regex is declared once per collectCompanions call, outside the inner for (const c of commits) loop, so its lastIndex carries between iterations. The explicit SQUASH.lastIndex = 0 reset handles this, but the pattern is now inconsistent with the matchAll approach adopted for TRAILER and REF in this same PR. Converting to matchAll would remove the need for the manual reset and the mutable let m variable.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

  2. .github/workflows/companion-pr-check.yml, line 113-114 (link)

    P2 Stale has-companion label not migrated

    The clear() function now removes LABEL (the new per-repo directional label) but never touches the old has-companion label that the previous workflow version applied. Any PR that was processed by the old workflow will keep has-companion stuck on it indefinitely — the new workflow won't clean it up even after all companions merge. A one-time manual sweep or a temporary call to removeLabel('has-companion') inside clear() would handle the migration.

Reviews (1): Last reviewed commit: "refactor(ci): stateless companion parseC..." | Re-trigger Greptile

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.

1 participant