Skip to content

Conversation

@chetanbothra
Copy link
Contributor

@chetanbothra chetanbothra commented Oct 1, 2025

Description

Types of changes

What types of changes does your code introduce?

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Other (please describe):

Further comments (optional)

Summary by CodeRabbit

  • Tests

    • Standardized end-to-end testing to run automatically on the master branch with a clearer, consistent flow.
  • Chores

    • Renamed the main workflow for clarity and standardized job naming.
    • Applied branch filters so install, e2e tests, and publishing run only on master.
    • Removed Slack failure notifications from CI.
    • Minor formatting and structural cleanups in the CI workflow for consistency.

@coderabbitai
Copy link

coderabbitai bot commented Oct 1, 2025

Walkthrough

The CircleCI config was updated to always run E2E tests (with master-branch filters), remove Slack failure notifications, rename the workflow to install-and-publish, add branch filters to publishing jobs, and expand the e2e job to check out a secondary repo on master and seed environment variables.

Changes

Cohort / File(s) Summary of Changes
CI workflow
.circleci/config.yml
Removed run parameter gating for e2e; removed Slack failure notification; renamed workflow to install-and-publish; added master branch filters to e2e-tests, publish-npm-package, and publish-github-release; expanded e2e job to checkout e2e-sdk-modular on master and perform env substitutions; minor structural/formatting updates.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Dev as Push to repo
    participant CircleCI as CircleCI Workflow
    participant E2E as e2e-tests job
    participant NPM as publish-npm-package
    participant GH as publish-github-release
    Note over Dev,CircleCI: Trigger on master branch only for gated jobs

    Dev->>CircleCI: Commit pushed
    alt branch == master
        CircleCI->>E2E: Run testnet-e2e-tests
        Note over E2E: Checkout e2e repo (master), seed env, run tests
        E2E-->>CircleCI: Status
        CircleCI->>NPM: Publish package (on success and filters)
        NPM-->>CircleCI: Status
        CircleCI->>GH: Create GitHub release (on success and filters)
        GH-->>CircleCI: Status
    else Non-master branch
        Note over CircleCI: Non-filtered jobs skipped (e2e, publish)
    end
    Note right of CircleCI: Slack failure notifications removed
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I thump my paw—pipelines align,
Master-bound tests now always chime.
No Slack to squeal, just logs that sing,
A tidy hop through every ring.
Checkout, seed, then publish, whee!
A hare-brained glide through CI. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The description includes only the template headings with placeholder hyphens and leaves out any actual summary of changes, detailed explanations, or checked change-type boxes, so it provides no useful information about what this pull request accomplishes. Please complete the template by adding a concise summary of the configuration changes, detailing each modification under “Description,” marking the appropriate checkboxes for the types of changes made, and including any relevant context or links under “Further comments.”
Title Check ❓ Inconclusive The title “Update config.yml” merely states that a configuration file was modified without indicating the nature or purpose of the changes, making it too generic to convey the primary update in this pull request. Please revise the title to succinctly describe the main change—for example, “Unconditionally run E2E tests and enforce master-branch gating in CircleCI config”—so that reviewers immediately understand the core update.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch PRO-e2e

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.

@github-actions
Copy link

github-actions bot commented Oct 1, 2025

Yooo! You forgot to bump the version in package.json!

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4dd7a5a and 9fc98c9.

📒 Files selected for processing (1)
  • .circleci/config.yml (3 hunks)

Comment on lines +74 to +88
sed -i.bak "s/private_key/$PRIVATE_KEY/g" .env
sed -i.bak "s/project_key/$PROJECT_KEY/g" .env
sed -i.bak "s/project_key_testnet/$PROJECT_KEY_TESTNET/g" .env
sed -i.bak "s/api_key/$API_KEY/g" .env
sed -i.bak "s/incorrect_api_key_arka/$INCORRECT_API_KEY_ARKA/g" .env
sed -i.bak "s/invalid_api_key_arka/$INVALID_API_KEY_ARKA/g" .env
sed -i.bak "s/bundler_api_key/$BUNDLER_API_KEY/g" .env
sed -i.bak "s/data_api_key/$DATA_API_KEY/g" .env
sed -i.bak "s/api_key_arka/$API_KEY_ARKA/g" .env
sed -i.bak "s/api_key_sessionkey/$API_KEY_SESSIONKEY/g" .env
sed -i.bak "s/primary_private_key/$PRIMARY_PRIVATE_KEY/g" .env
sed -i.bak "s/secondary_private_key/$SECONDARY_PRIVATE_KEY/g" .env
sed -i.bak "s/primary_wallet_address/$PRIMARY_WALLET_ADDRESS/g" .env
sed -i.bak "s/secondary_wallet_address/$SECONDARY_WALLET_ADDRESS/g" .env
sed -i.bak "s/paymaster_address/$PAYMASTER_ADDRESS/g" .env
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use safe templating for .env secrets

These sed -i replacements break as soon as any secret contains / or & (common in Base64 or URL-style API keys), corrupting .env and failing the run. Switch to a tool that handles arbitrary values—e.g. envsubst, jq on a JSON template, or python -c to rewrite the file—so the pipeline is robust to real-world secrets.

🤖 Prompt for AI Agents
In .circleci/config.yml around lines 74 to 88 the series of sed -i replacements
are unsafe because sed delimiters break when secrets contain characters like /
or &, so replace this block with a safe templating step that consumes
environment variables and writes .env atomically (for example: use envsubst with
${VAR} placeholders after exporting the CI variables, or run a short Python
script that reads a .env.template and replaces placeholders using os.environ
then writes the output); ensure the chosen approach preserves all variable
values verbatim, handles missing variables explicitly, and writes the final .env
file without relying on sed in-place editing.

Comment on lines +161 to +190
filters:
branches:
only:
- master

- e2e-tests:
name: testnet-e2e-tests
context: general-vars
requires:
- install
# filters:
# branches:
# only:
# - master
filters:
branches:
only:
- master

- publish-npm-package:
context: general-vars
requires:
- e2e-tests
- testnet-e2e-tests
filters:
branches:
only:
- master
- master

- publish-github-release:
context: general-vars
requires:
- publish-npm-package
filters:
branches:
only:
- master
- master
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Branch filters disable CI on non-master branches

All jobs now filter to master, so pushes/PRs on any other branch will execute no jobs and silently skip installs/tests/publishing. That removes CI coverage for feature branches and PR validation. Please drop these filters or move them to only the publishing steps so CI still runs on non-master refs.

🤖 Prompt for AI Agents
In .circleci/config.yml around lines 161 to 190 the branch filters restricting
all jobs to only the master branch cause CI to be disabled for non-master
branches; remove the shared "filters: branches: only: - master" blocks from the
common/job definitions so normal build/test jobs run on PRs and feature
branches, and instead apply branch-only filters only to the actual
publishing/deployment jobs (e.g., publish-npm-package and
publish-github-release) so publish steps remain master-only while installs/tests
remain enabled for all branches.

@sankhavaramsaitulasiram sankhavaramsaitulasiram merged commit f358566 into master Oct 2, 2025
2 of 3 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Oct 22, 2025
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants