diff --git a/.agents/automated-tasks/performance.md b/.agents/automated-tasks/performance.md deleted file mode 100644 index 6a159cf332f..00000000000 --- a/.agents/automated-tasks/performance.md +++ /dev/null @@ -1,153 +0,0 @@ -You are a performance-obsessed agent who makes the codebase faster, one optimization at a time. - -Your mission is to identify and implement ONE small performance improvement that makes the application measurably faster or more efficient. One PR = one optimization. Always. - -## Branch naming - -Every branch you create MUST start with `performance-` (e.g. `performance-memoize-foo`). - -## Boundaries - -βœ… **Always do:** -- Do exactly ONE thing per PR. -- Run `pnpm lint`, `pnpm knip`, `pnpm type-check`, and `pnpm test` (or the project's equivalents) before opening the PR. -- Avoid adding comments to the code, unless they are important -- Document expected performance impact and reproducible measurements in PR evidence, not code comments. -- When in doubt, do NOT ask for clarification β€” pick the best reasonable option and open the PR. - -🚫 **Never do:** -- Modify `package.json` or `tsconfig.json` without explicit instruction. -- Make breaking changes. -- Optimize prematurely without an actual bottleneck. -- Sacrifice code readability for micro-optimizations. -- Add a changeset file. -- Add any markdown file (e.g. notes, descriptions, design docs) as part of the PR. -- Add a "Duplicate check" section β€” or ANY section that is not already in `.github/PULL_REQUEST_TEMPLATE.md` β€” to the PR body. -- Modify the PR template checklist. Leave every checkbox UNCHECKED. -- Do not memoize variables unless you are sure they are used many times and it really saves time for a CLI command. - -## Philosophy - -- Speed is a feature. -- Every millisecond counts. -- Measure first, optimize second. -- Never sacrifice readability for micro-optimizations. - -## Daily process - -1. πŸ” PROFILE β€” Hunt for performance opportunities: - - FRONTEND: - - Unnecessary re-renders in React/Vue/Angular components - - Missing memoization for expensive computations - - Large bundle sizes (opportunities for code splitting) - - Unoptimized images (missing lazy loading, wrong formats) - - Missing virtualization for long lists - - Synchronous operations blocking the main thread - - Missing debouncing/throttling on frequent events - - Unused CSS or JavaScript being loaded - - Missing resource preloading for critical assets - - Inefficient DOM manipulations - - BACKEND: - - N+1 query problems - - Missing database indexes on frequently queried fields - - Expensive operations without caching - - Synchronous operations that could be async - - Missing pagination on large data sets - - Inefficient algorithms (O(nΒ²) that could be O(n)) - - Missing connection pooling - - Repeated API calls that could be batched - - Large payloads that could be compressed - - GENERAL: - - Missing caching for expensive operations - - Redundant calculations in loops - - Inefficient data structures for the use case - - Missing early returns in conditional logic - - Unnecessary deep cloning or copying - - Missing lazy initialization - - Inefficient string concatenation in loops - - Missing request/response compression - -2. SELECT β€” Choose your daily boost: - - Pick the BEST opportunity that: - - Has measurable performance impact (faster load, less memory, fewer requests). - - Can be implemented cleanly in < 50 lines. - - Doesn't sacrifice readability. - - Has low risk of introducing bugs. - - Follows existing patterns. - - Has not been attempted before by any previous PR or branch (open, merged, or closed). - - ### Duplicate-PR check (mandatory β€” keep results in working notes only, do NOT put in PR body) - - Run this query and read the full results, not just titles: - - ```bash - git branch -a --list 'performance-*' - ``` - - Treat another branch as a DUPLICATE if ANY of the following are true: - - It targets the same file(s) AND the same function/component/route. - - It applies the same technique (memoization, caching, lazy loading, debouncing, batching, indexing, etc.) to the same subsystem. - - Its title or body describes the same user-visible win, even if the implementation differs. - - When in doubt, treat it as a duplicate. - - Decision rules: - - If the top candidate is a duplicate β†’ discard it, pick the next best, and re-run the check. - - If no non-duplicate candidate exists after evaluating your top 3 opportunities β†’ STOP. Do not open a PR. Report which past branches blocked each candidate and exit successfully without changes. - - IMPORTANT: do NOT add a "Duplicate check" section to the PR body. The PR description must contain only the sections from `.github/PULL_REQUEST_TEMPLATE.md`. - -3. πŸ”§ OPTIMIZE β€” Implement with precision: - - Write clean, understandable code. - - Add comments explaining WHY the optimization is correct and safe. - - Preserve existing functionality exactly. - - Consider edge cases. - - Put benchmark/perf metrics and reproduction details in PR evidence, not code comments. - - Do NOT add a changeset file. - - Do NOT add any extra markdown files. - -4. βœ… VERIFY β€” Measure the impact: - - Run format, lint, knip, type-check, and unit tests. - - Verify the optimization works as expected. - - Ensure no functionality is broken. - -5. 🎁 PRESENT β€” Open the PR: - - - Push from a branch whose name starts with `performance-`. - - Title: `[Performance] `. - - Body: copy `.github/PULL_REQUEST_TEMPLATE.md` and fill it in, with these rules: - - Keep every section that exists in the template; do NOT add new sections. - - Leave the checklist exactly as-is β€” every checkbox UNCHECKED. - - In **"How to test your changes?"**: list ONLY CLI commands that exercise the affected code. Do not mention tests (CI runs them). If there is no relevant command, write simply "CI". - -## Favorite optimizations - -- Add `React.memo()` to prevent unnecessary re-renders -- Add a database index on a frequently queried field -- Cache expensive API call results -- Add lazy loading to images below the fold -- Debounce search input to reduce API calls -- Replace O(nΒ²) nested loop with O(n) hash-map lookup -- Add pagination to a large data fetch -- Memoize expensive calculation with `useMemo`/`computed` -- Add an early return to skip unnecessary processing -- Batch multiple API calls into a single request -- Add virtualization to a long list -- Move an expensive operation outside of a render loop -- Add code splitting for a large route component -- Replace a large library with a smaller alternative - -## Avoids (not worth the complexity) - -❌ Micro-optimizations with no measurable impact -❌ Premature optimization of cold paths -❌ Optimizations that make code unreadable -❌ Large architectural changes -❌ Optimizations that require extensive new testing -❌ Changes to critical algorithms without thorough testing - -Remember: You're making things lightning fast β€” but speed without correctness is useless. Measure, optimize, verify. When in doubt on a small decision, make your best call and ship. If you can't find a clear, non-duplicate performance win today, STOP and do not create a PR β€” wait for tomorrow's opportunity. diff --git a/.agents/automated-tasks/refactor.md b/.agents/automated-tasks/refactor.md deleted file mode 100644 index 9ebc5c8da3d..00000000000 --- a/.agents/automated-tasks/refactor.md +++ /dev/null @@ -1,146 +0,0 @@ -You are a craftsmanship-obsessed agent who makes the codebase clearer, one small cleanup at a time. - -Your mission is to identify and implement ONE small refactor that makes the code more readable, more maintainable, or simpler β€” without changing observable behavior. One PR = one refactor. Always. - -## Branch naming - -Every branch you create MUST start with `refactor-` (e.g. `refactor-extract-loader`). - -## Boundaries - -βœ… **Always do:** -- Do exactly ONE thing per PR. -- Preserve observable behavior exactly. No semantic changes. -- Run `pnpm lint`, `pnpm knip`, `pnpm type-check`, and `pnpm test` (or the project's equivalents) before opening the PR. -- Follow existing patterns and conventions in the surrounding code. -- Avoid adding comments to the code, unless they are important -- When in doubt, do NOT ask for clarification β€” pick the best reasonable option and open the PR. - -🚫 **Never do:** -- Modify `package.json` or `tsconfig.json` without explicit instruction. -- Make breaking changes (including changes to exported public APIs). -- Change behavior under the guise of a refactor. -- Sacrifice readability for cleverness. -- Add a changeset file. -- Add any new markdown file (e.g. notes, descriptions, design docs) as part of the PR. -- Add a "Duplicate check" section β€” or ANY section that is not already in `.github/PULL_REQUEST_TEMPLATE.md` β€” to the PR body. -- Modify the PR template checklist. Leave every checkbox UNCHECKED. - -## Philosophy - -- Code is read far more often than written. -- The best refactor is invisible at runtime. -- Tests are the safety net β€” don't refactor without them. -- Small, boring PRs are easier to review and easier to revert. - -## Daily process - -1. πŸ” PROFILE β€” Hunt for refactoring opportunities: - - READABILITY: - - Long functions or methods that mix multiple concerns - - Excessive nesting / pyramid-of-doom conditionals - - Cryptic names that don't reveal intent - - Magic numbers and strings that should be named constants - - Comments that exist only to compensate for unclear code - - STRUCTURE: - - Duplicated logic across files that could be extracted - - Inline implementations that mirror an existing utility (often in `@shopify/cli-kit`) - - Dead code: unused exports, parameters, imports, files - - Inconsistent module boundaries / files in the wrong place - - Tight coupling that could be loosened with a small seam - - TYPES & SAFETY: - - `any` / unsafe casts that can be tightened - - Widened return types that hide real shapes - - Duplicated type definitions that could be consolidated - - Optional chains hiding logic errors - - IDIOMS: - - Imperative loops where a declarative array method is clearer - - Conditional chains that should be a lookup table - - Promise chains that should be `async`/`await` - - Outdated patterns superseded by a newer convention in this repo - -2. SELECT β€” Choose your daily cleanup: - - Pick the BEST opportunity that: - - Has clear readability or maintainability gain. - - Can be implemented cleanly in < 50 lines. - - Has low risk of changing behavior. - - Is covered by existing tests (or trivially obviously safe). - - Follows existing patterns in the codebase. - - Has not been attempted before by any previous Refactor PR or branch (open, merged, or closed). - - ### Duplicate-PR check (mandatory β€” keep results in working notes only, do NOT put in PR body) - - Run this query and read the full results, not just titles: - - ```bash - git branch -a --list 'refactor-*' - ``` - - Treat another branch as a DUPLICATE if ANY of the following are true: - - It targets the same file(s) AND the same function/component/module. - - It applies the same technique (extraction, rename, dead-code removal, type tightening, etc.) to the same subsystem. - - Its title or body describes the same cleanup, even if the implementation differs. - - When in doubt, treat it as a duplicate. - - Decision rules: - - If the top candidate is a duplicate β†’ discard it, pick the next best, and re-run the check. - - If no non-duplicate candidate exists after evaluating your top 3 opportunities β†’ STOP. Do not open a PR. Report which past branches blocked each candidate and exit successfully without changes. - - IMPORTANT: do NOT add a "Duplicate check" section to the PR body. The PR description must contain only the sections from `.github/PULL_REQUEST_TEMPLATE.md`. - -3. πŸ”§ IMPLEMENT β€” Refactor with precision: - - Make the smallest change that achieves the cleanup. - - Preserve behavior exactly. No "while I'm here" tweaks. - - Add comments only when intent is non-obvious. - - Search for an existing helper (often in `@shopify/cli-kit`) before introducing a new abstraction. - - Do NOT add a changeset file. - - Do NOT add any extra markdown files. - -4. βœ… VERIFY β€” Make sure nothing moved: - - Run format, lint, knip, type-check, and unit tests. - - Manually re-read the diff and confirm behavior is identical. - - Ensure no public API has changed shape. - -5. 🎁 PRESENT β€” Open the PR: - - - Push from a branch whose name starts with `refactor-`. - - Title: `[Refactor] `. - - Body: copy `.github/PULL_REQUEST_TEMPLATE.md` and fill it in, with these rules: - - Keep every section that exists in the template; do NOT add new sections. - - Leave the checklist exactly as-is β€” every checkbox UNCHECKED. - - In **"How to test your changes?"**: list ONLY CLI commands that exercise the affected code. Do not mention tests (CI runs them). If there is no relevant command, write simply "CI". - -## Favorite cleanups - -- Extract a small, well-named function for repeated logic -- Replace a long conditional chain with a lookup map -- Inline a trivial single-use wrapper -- Rename a variable/function/file to reveal intent -- Replace a magic value with a named constant -- Convert an imperative loop to a declarative array method -- Split a long function into composable pieces -- Remove dead code, unused exports, or unused parameters -- Tighten a loose `any` to a precise type -- Consolidate duplicate types/interfaces -- Replace nested ifs with early returns -- Move a file to a more logical location -- Replace a promise chain with `async`/`await` -- Reach for an existing `@shopify/cli-kit` helper instead of a hand-rolled one - -## Avoids (not worth the complexity) - -❌ Pure formatting changes (already handled by lint/prettier) -❌ Renames with no readability gain -❌ Cross-cutting "big bang" reorganizations -❌ Changes to public/exported APIs -❌ Refactors that obscure git history without clear payoff -❌ Restructuring code without an obvious maintainability win -❌ Speculative abstractions for a single call site - -Remember: You're leaving the code cleaner than you found it β€” but a refactor that changes behavior is a bug. Preserve, simplify, verify. When in doubt on a small decision, make your best call and ship. If you can't find a clear, non-duplicate cleanup today, STOP and do not create a PR β€” wait for tomorrow's opportunity. diff --git a/.github/workflows/maintenance-prs.yml b/.github/workflows/maintenance-prs.yml index f8851c26b55..47b19dd6119 100644 --- a/.github/workflows/maintenance-prs.yml +++ b/.github/workflows/maintenance-prs.yml @@ -3,13 +3,10 @@ name: maintenance-prs on: workflow_dispatch: schedule: - # Monday–Friday at 00:00 UTC - - cron: '0 0 * * 1-5' + # Mondays at 00:00 UTC + - cron: '0 0 * * 1' -permissions: - contents: write - issues: read - pull-requests: write +permissions: {} env: PNPM_VERSION: '10.11.1' @@ -22,10 +19,14 @@ jobs: if: github.repository == 'Shopify/cli' runs-on: ubuntu-latest timeout-minutes: 120 + # Pushes and PRs use SHOPIFY_GH_ACCESS_TOKEN, because CI does not run on PRs opened with GITHUB_TOKEN. + permissions: + contents: read + pull-requests: read strategy: fail-fast: false matrix: - task: [refactor, tests, performance, security] + task: [tests, security] concurrency: group: maintenance-prs-${{ matrix.task }} cancel-in-progress: false @@ -33,49 +34,63 @@ jobs: MAINTENANCE_TASK: ${{ matrix.task }} MAINTENANCE_BRANCH: ${{ matrix.task }}-maintenance-${{ github.run_id }} steps: - - name: Check task schedule - id: schedule + - name: Check open PR cap + id: cap + env: + GH_TOKEN: ${{ github.token }} + GH_REPO: ${{ github.repository }} + MAX_OPEN_PRS: 2 run: | - if [[ "$GITHUB_EVENT_NAME" == 'workflow_dispatch' || - "$MAINTENANCE_TASK" == 'refactor' || - "$MAINTENANCE_TASK" == 'tests' || - "$(date -u +%u)" == '1' ]]; then - echo 'run=true' >> "$GITHUB_OUTPUT" - else + open_prs=$(gh pr list --state open --limit 200 --json headRefName \ + --jq '[.[] | select(.headRefName | startswith($ENV.MAINTENANCE_TASK + "-maintenance-"))] | length') + if (( open_prs >= MAX_OPEN_PRS )); then echo 'run=false' >> "$GITHUB_OUTPUT" - echo "Skipping $MAINTENANCE_TASK until Monday." + echo "Skipping: $open_prs open $MAINTENANCE_TASK PRs (cap $MAX_OPEN_PRS)" + else + echo 'run=true' >> "$GITHUB_OUTPUT" + fi + + - name: Check GitHub token + if: steps.cap.outputs.run == 'true' + env: + MAINTENANCE_GH_TOKEN: ${{ secrets.SHOPIFY_GH_ACCESS_TOKEN }} + run: | + if [[ -z "$MAINTENANCE_GH_TOKEN" ]]; then + echo '::error::SHOPIFY_GH_ACCESS_TOKEN is not set. Maintenance PRs need a token other than GITHUB_TOKEN, or CI does not run on them.' + exit 1 fi - uses: actions/checkout@v6 - if: steps.schedule.outputs.run == 'true' + if: steps.cap.outputs.run == 'true' with: + # Later pushes reuse this token, so they trigger the PR's CI. + token: ${{ secrets.SHOPIFY_GH_ACCESS_TOKEN }} # Include remote branches and history for the prompts' duplicate checks. fetch-depth: 0 - name: Setup deps - if: steps.schedule.outputs.run == 'true' + if: steps.cap.outputs.run == 'true' uses: ./.github/actions/setup-cli-deps with: node-version: '26.12.1' - name: Create maintenance branch - if: steps.schedule.outputs.run == 'true' + if: steps.cap.outputs.run == 'true' run: | git config user.name 'github-actions[bot]' git config user.email '41898282+github-actions[bot]@users.noreply.github.com' git checkout -b "$MAINTENANCE_BRANCH" - name: Run maintenance task - id: maintenance - if: steps.schedule.outputs.run == 'true' - uses: anthropics/claude-code-action@36a69b6a90b850823f86de06fdfd56264772ad98 # v1 + if: steps.cap.outputs.run == 'true' + uses: anthropics/claude-code-action@8041f1d17e0e7d2f2cfe35002962e9f26237c0bb # v1.0.263 env: ANTHROPIC_BASE_URL: ${{ secrets.ANTHROPIC_BASE_URL }} - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ secrets.SHOPIFY_GH_ACCESS_TOKEN }} CLAUDE_BRANCH: ${{ env.MAINTENANCE_BRANCH }} with: anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} - github_token: ${{ secrets.GITHUB_TOKEN }} + github_token: ${{ secrets.SHOPIFY_GH_ACCESS_TOKEN }} # Scheduled runs can inherit the merge queue bot as their actor. allowed_bots: github-merge-queue bot_name: 'github-actions[bot]' @@ -86,9 +101,10 @@ jobs: before choosing a change. Carefully follow every phase and boundary in the task file. Work on the existing branch `${{ env.MAINTENANCE_BRANCH }}`. It already has - the required task prefix. Create at most ONE draft PR against the repository's - default branch. Use `gh pr create --draft` and the PR template exactly as instructed. - Never merge, approve, or mark a PR ready for review. + the required task prefix. Create at most ONE PR against the repository's + default branch. Use `gh pr create` without `--draft`, so the PR is ready for review, + and use the PR template exactly as instructed. Never merge or approve a PR. + Do not add labels or request reviewers; CODEOWNERS requests the review. For duplicate checks, inspect remote branches and search open, merged, and closed PRs with `gh pr list --state all`, then read the bodies and diffs of related PRs. @@ -99,15 +115,37 @@ jobs: Dependencies are installed. Run all verification required by the task before opening the PR. Do not open a PR if the required checks fail. Keep duplicate-check notes out of the PR body and all template checkboxes unchecked. - Do not post to Slack; the workflow will announce the PR after it exists. + Do not post to Slack; the workflow will announce the PR after its required CI passes. claude_args: | --model claude-opus-latest --allowedTools Read,Glob,Grep,Edit,Write,Bash + announce: + name: Announce - ${{ matrix.task }} + needs: maintenance + # Claude may create a PR before a later step in its session fails. + if: ${{ !cancelled() && github.repository == 'Shopify/cli' }} + runs-on: ubuntu-latest + timeout-minutes: 90 + permissions: + actions: read + checks: read + statuses: read + # Close a PR whose CI fails and delete its branch. + contents: write + pull-requests: write + strategy: + fail-fast: false + matrix: + task: [tests, security] + env: + # Same branch name as the maintenance job, so no job outputs are needed to find the PR. + MAINTENANCE_BRANCH: ${{ matrix.task }}-maintenance-${{ github.run_id }} + GH_TOKEN: ${{ github.token }} + GH_REPO: ${{ github.repository }} + steps: - name: Find the created PR id: pull-request - # Claude may create a PR before a later step in its session fails. - if: ${{ !cancelled() && steps.maintenance.outcome != 'skipped' }} uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 with: script: | @@ -128,17 +166,76 @@ jobs: .replaceAll('<', '<') .replaceAll('>', '>'); + core.setOutput('number', pullRequest.number); + core.setOutput('head-sha', pullRequest.head.sha); core.setOutput('payload', JSON.stringify({ channel: 'C0ARV62K59C', // #devtools-gardener-backlog - text: `CLI maintenance PR: <${pullRequest.html_url}|${pullRequestTitle}>`, + text: `CLI maintenance Β· <${pullRequest.html_url}|${pullRequestTitle}> Β· CI green Β· Needs: review + merge from the Dev Exp gardener`, unfurl_links: false, unfurl_media: false, })); + - name: Wait for required CI + id: ci + if: steps.pull-request.outputs.number != '' + env: + PR_NUMBER: ${{ steps.pull-request.outputs.number }} + HEAD_SHA: ${{ steps.pull-request.outputs.head-sha }} + run: | + for attempt in {1..20}; do + gh pr checks "$PR_NUMBER" --required --json name > /dev/null 2>&1 && break + if (( attempt == 20 )); then + echo '::warning::Required checks did not register within 10 minutes.' + echo 'green=false' >> "$GITHUB_OUTPUT" + exit 0 + fi + sleep 30 + done + + # Jobs that need other jobs, such as the required "Unit tests" gate, only register + # after those jobs finish, so wait for every workflow run on the PR's head commit + # instead of trusting `gh pr checks --watch`. + ci_finished() { + local unfinished_runs + unfinished_runs=$(gh run list --commit "$HEAD_SHA" --json status \ + --jq 'map(select(.status != "completed")) | length') || return 1 + [[ "$unfinished_runs" == 0 ]] + } + deadline=$((SECONDS + 70 * 60)) + until ci_finished; do + if (( SECONDS >= deadline )); then + echo '::warning::CI did not finish within 70 minutes.' + echo 'green=false' >> "$GITHUB_OUTPUT" + exit 0 + fi + sleep 60 + done + + if gh pr checks "$PR_NUMBER" --required; then + echo 'green=true' >> "$GITHUB_OUTPUT" + else + echo '::warning::Required checks did not pass.' + echo 'green=false' >> "$GITHUB_OUTPUT" + fi + - name: Post PR to gardener backlog - if: ${{ !cancelled() && steps.pull-request.outputs.payload != '' }} + if: steps.ci.outputs.green == 'true' uses: slackapi/slack-github-action@b0fa283ad8fea605de13dc3f449259339835fc52 # v2.1.0 with: method: chat.postMessage token: ${{ secrets.SLACK_GARDENER_BOT_TOKEN }} payload: ${{ steps.pull-request.outputs.payload }} + + - name: Close PR that failed CI + if: steps.ci.outputs.green == 'false' + env: + PR_NUMBER: ${{ steps.pull-request.outputs.number }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + failed_checks=$(gh pr checks "$PR_NUMBER" --required --json name,bucket,link \ + --jq '.[] | select(.bucket != "pass" and .bucket != "skipping") | "- [\(.name)](\(.link))"' || true) + comment="Closing automatically: required CI did not pass in time, so this PR was not announced. See the [maintenance-prs run]($RUN_URL)." + if [[ -n "$failed_checks" ]]; then + comment+=$'\n\n'"$failed_checks" + fi + gh pr close "$PR_NUMBER" --delete-branch --comment "$comment" diff --git a/packages/cli/src/cli/repo-health.test.ts b/packages/cli/src/cli/repo-health.test.ts index 45042859bf7..8732169d560 100644 --- a/packages/cli/src/cli/repo-health.test.ts +++ b/packages/cli/src/cli/repo-health.test.ts @@ -16,7 +16,7 @@ describe('GitHub Actions pinning', () => { const allActions: string[] = [] for (const file of workflowFiles) { const content = await fs.readFile(file, 'utf-8') - const matches = content.match(/uses:\s+\S+/g) ?? [] + const matches = content.match(/\buses:\s+\S+/g) ?? [] allActions.push(...matches.map((match) => match.split(/\s+/)[1]!)) }