Skip to content

ci: de-duplicate ci.yaml and qa.yaml (JG-18) - #41

Merged
mobileskyfi merged 2 commits into
mainfrom
ci/dedupe-workflows
Jun 16, 2026
Merged

ci: de-duplicate ci.yaml and qa.yaml (JG-18)#41
mobileskyfi merged 2 commits into
mainfrom
ci/dedupe-workflows

Conversation

@mobileskyfi

@mobileskyfi mobileskyfi commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

What

Removes two file-visible CI overlaps, no behavior loss (the JG-18 "audit + de-dupe" foundation).

Overlap Before After
chr-integration defined in both ci.yaml and qa.yaml → a single workflow_dispatch fired two identical CHR jobs consolidated to qa.yaml (the only workflow with the weekly schedule + the stable/long-term channel matrix)
build ran in ci.yaml (ci) and qa.yaml (extended-tests) only in the ci.yaml fast gate

Result

  • ci.yaml = the fast push/PR gate: lint, lint:ci, unit test, build, fixture-integration (PR). Its CHR-only workflow_dispatch inputs are removed.
  • qa.yaml = the heavier/scheduled surface: coverage (test:ci), CodeQL, AI-findings probe, and the CHR matrix (schedule + dispatch).
  • On-demand CHR is now gh workflow run qa.yaml -f run-integration=true -f routeros_channel=<channel> (was ci.yaml).

Security-scan note (the "too many duplicate scans" concern)

code-scanning/default-setup is not-configured, so the qa.yaml workflow CodeQL (javascript-typescript + actions — two distinct languages, not dupes) is the only code scan. No duplicate security scan to remove.

CI-config only; no source changes. YAML validated locally; lint:ci green. The qa.yaml chr-integration block is unchanged (only the duplicate build step was dropped).

Part of June-Gloom Phase 2 (CI). JG-18 (de-dupe portion).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Optimized CI/CD workflows: CI pipeline now triggers exclusively on main branch pushes and pull requests.
    • Consolidated RouterOS CHR integration testing from CI into the dedicated QA workflow.
    • Removed redundant build steps from QA extended tests for streamlined operations.

Two file-visible overlaps removed, no behavior loss:

- `chr-integration` was defined in BOTH ci.yaml and qa.yaml, so a single
  workflow_dispatch fired two identical CHR jobs. Consolidated to qa.yaml — the
  only workflow with the weekly schedule + the stable/long-term channel matrix.
  ci.yaml is now purely the fast push/PR gate (lint, unit test, build, fixture
  integration); its CHR-only workflow_dispatch inputs are removed. On-demand CHR
  is now `gh workflow run qa.yaml -f run-integration=true -f routeros_channel=…`.
- `build` ran in both ci.yaml (`ci`) and qa.yaml (`extended-tests`). Dropped from
  qa.yaml; build stays in the fast gate.

CodeQL default-setup is not-configured, so the qa.yaml workflow CodeQL
(javascript-typescript + actions, two distinct languages) is the only security
scan — no duplicate to remove there.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 16, 2026 02:53
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@mobileskyfi, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 56 minutes and 18 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1c9e1dd8-c59b-4620-b1e5-5d7f4b1e26b5

📥 Commits

Reviewing files that changed from the base of the PR and between b679940 and 2b3ee00.

📒 Files selected for processing (1)
  • .github/workflows/ci.yaml
📝 Walkthrough

Walkthrough

The CI workflow (ci.yaml) removes its workflow_dispatch trigger, associated CHR integration inputs, and the chr-integration job entirely, adding comments that CHR integration lives only in qa.yaml. The QA workflow (qa.yaml) removes the bun run build step from its extended-tests job and adds comments attributing build ownership to ci.yaml.

Changes

Workflow Responsibility Consolidation

Layer / File(s) Summary
Remove chr-integration from ci.yaml and build step from qa.yaml
.github/workflows/ci.yaml, .github/workflows/qa.yaml
ci.yaml drops workflow_dispatch, its CHR-related inputs, and the full chr-integration job; header comments note CHR integration is exclusive to qa.yaml. qa.yaml removes the bun run build step from extended-tests and adds inline comments indicating ci.yaml owns the build.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐇 Two workflows were tangled, doing each other's chores,
So I hopped through the YAML and closed the wrong doors.
CI runs fast now, just push and PR,
The CHR tests live in QA — that's where they are!
Clean lanes, tidy duties, no duplicates remain,
sniff sniff — smells like order in the pipeline again. 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description is comprehensive and well-structured, covering the what, before/after comparison, results, and addressing security concerns. However, it does not follow the provided template structure with sections like 'Links', 'Change type', and 'Notes'. Consider following the repository's PR template by adding sections for Linked spec ID, work item (JG-18), marking the 'Tooling/workflow change' checkbox, and explicitly documenting validation and assumptions.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main objective: deduplicating CI workflow configurations in ci.yaml and qa.yaml with reference to the work item JG-18.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/dedupe-workflows

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 and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR de-duplicates GitHub Actions CI configuration by consolidating the RouterOS CHR integration workflow into qa.yaml and keeping ci.yaml as the fast PR/push gate.

Changes:

  • Removed the duplicate build step from the QA workflow so build runs only in the CI fast gate.
  • Removed the duplicate CHR integration job and its dispatch inputs from ci.yaml, leaving CHR integration in qa.yaml.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
.github/workflows/qa.yaml Drops the duplicate build step from QA’s extended-tests job while retaining coverage, probes, CodeQL, and CHR matrix execution.
.github/workflows/ci.yaml Removes the CHR integration job and workflow_dispatch inputs so CI remains a fast push/PR gate.

Comment thread .github/workflows/ci.yaml
Comment on lines 3 to +10
on:
push:
branches: [main]
pull_request:
branches: [main]
workflow_dispatch:
inputs:
run-integration:
description: Run RouterOS CHR integration tests
type: boolean
default: true
routeros_channel:
description: RouterOS channel for CHR integration when routeros_version is empty
type: choice
options:
- stable
- long-term
- testing
- development
default: stable
routeros_version:
description: Explicit RouterOS version for CHR integration; overrides routeros_channel
type: string
default: ""
# CI is the fast push/PR gate (lint, unit tests, build, fixture integration).
# RouterOS CHR integration lives only in qa.yaml (scheduled matrix + on-demand
# workflow_dispatch) so there is one chr-integration definition, not two.
Copilot review on #41: removing workflow_dispatch entirely blocked manually
re-running the fast gate. Re-add a bare `workflow_dispatch: {}` (no inputs); only
the CHR-specific inputs + job move to qa.yaml, so "no behavior loss" holds.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@mobileskyfi
mobileskyfi merged commit 4557db1 into main Jun 16, 2026
9 checks passed
@mobileskyfi
mobileskyfi deleted the ci/dedupe-workflows branch June 16, 2026 03:05
mobileskyfi added a commit that referenced this pull request Jun 16, 2026
* test(smoke): add network-free CLI smoke tier (JG-10)

A real-subprocess smoke set over the cli-process harness: top-level + command
help, the invalid-command envelope, and a full devices CDB round-trip
(add → list → show → remove + an unknown-target error envelope) against a
temp-HOME default CDB. No RouterOS/CHR, so it is not gated behind
CENTRS_RUN_FAST_INTEGRATION and runs in the fast push/PR gate via the existing
`bun test` Test step — no new CI job, which would re-duplicate right after the
#41 workflow de-dupe. Adds a `test:smoke` script for standalone runs.

The pre-existing test/unit/smoke.test.ts is an exports/project-baseline unit
test, not a CLI smoke; the real CLI smoke needs the subprocess harness
(test/integration/cli-process.ts), hence test/integration/cli-smoke.test.ts.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(smoke): clarify layout + harden envelope parse (review)

Address PR #42 review:
- Copilot: document that test/integration/ "platform-specific" includes the
  network-free, always-run subprocess CLI smoke (it spawns the real cli.ts via
  cli-process.ts), so its placement matches the test layout rules.
- CodeRabbit: parseEnvelope now validates it got an object with a boolean `ok`
  and throws a clear message otherwise, instead of a bare type assertion.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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