chore: import upstream snapshot with attribution
CI / Run CI (push) Has been cancelled
CI / check-backend (push) Has been cancelled
CI / check-frontend (push) Has been cancelled
CI / tests (push) Has been cancelled
CI / e2e-tests (push) Has been cancelled
Copilot Setup Steps / copilot-setup-steps (push) Has been cancelled
CI / Run CI (push) Has been cancelled
CI / check-backend (push) Has been cancelled
CI / check-frontend (push) Has been cancelled
CI / tests (push) Has been cancelled
CI / e2e-tests (push) Has been cancelled
Copilot Setup Steps / copilot-setup-steps (push) Has been cancelled
This commit is contained in:
@@ -0,0 +1,426 @@
|
||||
# ESLint v9 Flat Config Migration + lint-staged Unification
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Migrate ESLint from v8 (`.eslintrc`) to v9 (flat config) so that `eslint` with no args defaults to `.` (all files), while explicit file args override this — enabling lint-staged to pass staged files via `pnpm --filter` without linting the entire package.
|
||||
|
||||
**Architecture:** Replace the JSON `.eslintrc` + `.eslintignore` + `cypress/.eslintrc` with a single `eslint.config.mjs` at the repo root. Update all package `lint` scripts to bare `eslint` (no `--ext`, no directory). Update `lint-staged.config.js` to use package-scoped globs with `pnpm --filter` for ESLint and type-check.
|
||||
|
||||
**Tech Stack:** ESLint 9, typescript-eslint (unified package), @eslint/js, eslint-plugin-cypress 6, eslint-plugin-chai-friendly
|
||||
|
||||
**Key behavior (verified empirically on this repo):**
|
||||
|
||||
- ESLint v9 flat config: `eslint` (no args) = lints `.` (current directory)
|
||||
- ESLint v9 flat config: `eslint file1.ts file2.ts` = lints **only** those files
|
||||
- ESLint v8: no such behavior exists (no args = 0 files, silent exit 0)
|
||||
|
||||
---
|
||||
|
||||
## File Map
|
||||
|
||||
| Action | File | Responsibility |
|
||||
| ------ | -------------------------------- | ------------------------------------------------------------------------------ |
|
||||
| Create | `eslint.config.mjs` | Single flat config replacing `.eslintrc`, `.eslintignore`, `cypress/.eslintrc` |
|
||||
| Delete | `.eslintrc` | Replaced by `eslint.config.mjs` |
|
||||
| Delete | `.eslintignore` | Merged into `ignores` array in `eslint.config.mjs` |
|
||||
| Delete | `cypress/.eslintrc` | Merged into `eslint.config.mjs` with `files: ['cypress/**/*.ts']` |
|
||||
| Modify | `package.json` | Update ESLint deps |
|
||||
| Modify | `frontend/package.json` | Remove `--ext` and `./src` from lint script |
|
||||
| Modify | `libs/react-client/package.json` | Remove `--ext` and `./src` from lint script |
|
||||
| Modify | `libs/copilot/package.json` | Remove `--ext` and `./src` from lint script |
|
||||
| Modify | `lint-staged.config.js` | Package-scoped globs with `pnpm --filter` |
|
||||
|
||||
---
|
||||
|
||||
## Task 1: Update Dependencies
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `package.json` (devDependencies)
|
||||
- **Step 1: Remove old ESLint packages and add new ones**
|
||||
|
||||
```bash
|
||||
pnpm remove @typescript-eslint/eslint-plugin @typescript-eslint/parser eslint eslint-plugin-cypress
|
||||
```
|
||||
|
||||
- **Step 2: Install ESLint v9 and new packages**
|
||||
|
||||
```bash
|
||||
pnpm add -Dw eslint@^9 @eslint/js typescript-eslint eslint-plugin-cypress@^6
|
||||
```
|
||||
|
||||
> `eslint-plugin-chai-friendly@^1.1.1` stays — its peer dep is `eslint>=3.0.0` and it exports `configs.recommendedFlat` (verified at runtime).
|
||||
|
||||
- **Step 3: Verify installation**
|
||||
|
||||
```bash
|
||||
npx eslint --version
|
||||
```
|
||||
|
||||
Expected: `v9.x.x`
|
||||
|
||||
- **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add package.json pnpm-lock.yaml
|
||||
git commit -m "chore: upgrade eslint to v9, add typescript-eslint unified package"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 2: Create Flat Config
|
||||
|
||||
**Files:**
|
||||
|
||||
- Create: `eslint.config.mjs`
|
||||
|
||||
This replaces three files: `.eslintrc`, `.eslintignore`, `cypress/.eslintrc`.
|
||||
|
||||
- **Step 1: Create `eslint.config.mjs`**
|
||||
|
||||
```js
|
||||
import eslint from '@eslint/js';
|
||||
import chaiFriendly from 'eslint-plugin-chai-friendly';
|
||||
import cypressPlugin from 'eslint-plugin-cypress/flat';
|
||||
import tseslint from 'typescript-eslint';
|
||||
|
||||
export default tseslint.config(
|
||||
// Global ignores (replaces .eslintignore + ignorePatterns)
|
||||
{
|
||||
ignores: ['**/node_modules/', '**/dist/', '**/*.jsx']
|
||||
},
|
||||
|
||||
// Base configs (replaces "extends" in .eslintrc)
|
||||
eslint.configs.recommended,
|
||||
...tseslint.configs.recommended,
|
||||
|
||||
// Project rules (replaces "rules" in .eslintrc)
|
||||
{
|
||||
linterOptions: {
|
||||
reportUnusedDisableDirectives: true
|
||||
},
|
||||
rules: {
|
||||
'@typescript-eslint/no-non-null-assertion': 'off',
|
||||
'@typescript-eslint/no-explicit-any': 'off',
|
||||
'no-unused-vars': 'off',
|
||||
'@typescript-eslint/no-unused-vars': [
|
||||
'error',
|
||||
{
|
||||
argsIgnorePattern: '^_',
|
||||
varsIgnorePattern: '^_',
|
||||
caughtErrorsIgnorePattern: '^_',
|
||||
ignoreRestSiblings: true
|
||||
}
|
||||
]
|
||||
}
|
||||
},
|
||||
|
||||
// Cypress overrides (replaces cypress/.eslintrc)
|
||||
{
|
||||
files: ['cypress/**/*.ts'],
|
||||
...cypressPlugin.configs.recommended,
|
||||
plugins: {
|
||||
'chai-friendly': chaiFriendly
|
||||
},
|
||||
rules: {
|
||||
...chaiFriendly.configs.recommendedFlat.rules
|
||||
}
|
||||
}
|
||||
);
|
||||
```
|
||||
|
||||
> **Why `.mjs`:** Root `package.json` has no `"type": "module"`, so ESM requires the `.mjs` extension.
|
||||
>
|
||||
> **What's gone:**
|
||||
>
|
||||
> - `"root": true` — flat config doesn't cascade from parent dirs
|
||||
> - `"parser"` — `tseslint.configs.recommended` sets the parser automatically
|
||||
> - `--ext .ts,.tsx` — flat config uses `files` patterns in config objects
|
||||
> - `.eslintignore` — replaced by `ignores` array
|
||||
> - `cypress/.eslintrc` — merged with `files: ['cypress/**/*.ts']`
|
||||
|
||||
- **Step 2: Verify the config loads**
|
||||
|
||||
```bash
|
||||
npx eslint --print-config frontend/src/App.tsx
|
||||
```
|
||||
|
||||
Expected: JSON output showing merged config with `@typescript-eslint/*` rules.
|
||||
|
||||
- **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add eslint.config.mjs
|
||||
git commit -m "chore: add eslint v9 flat config"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 3: Delete Old Config Files
|
||||
|
||||
**Files:**
|
||||
|
||||
- Delete: `.eslintrc`
|
||||
- Delete: `.eslintignore`
|
||||
- Delete: `cypress/.eslintrc`
|
||||
- **Step 1: Delete the old files**
|
||||
|
||||
```bash
|
||||
rm .eslintrc .eslintignore cypress/.eslintrc
|
||||
```
|
||||
|
||||
- **Step 2: Verify ESLint still works with flat config only**
|
||||
|
||||
```bash
|
||||
npx eslint frontend/src/App.tsx
|
||||
```
|
||||
|
||||
Expected: no errors (or only pre-existing lint errors, not config errors).
|
||||
|
||||
- **Step 3: Commit**
|
||||
|
||||
```bash
|
||||
git add .eslintrc .eslintignore cypress/.eslintrc
|
||||
git commit -m "chore: remove legacy eslintrc config files"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 4: Update Package Lint Scripts
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `frontend/package.json:10` — `"lint"` script
|
||||
- Modify: `libs/react-client/package.json:9` — `"lint"` script
|
||||
- Modify: `libs/copilot/package.json:10` — `"lint"` script
|
||||
|
||||
In flat config, `eslint` with no args defaults to `.` and file type matching is handled by config objects (no `--ext` needed).
|
||||
|
||||
- **Step 1: Update frontend lint script**
|
||||
|
||||
In `frontend/package.json`, change:
|
||||
|
||||
```diff
|
||||
- "lint": "eslint ./src --ext .ts,.tsx",
|
||||
+ "lint": "eslint",
|
||||
```
|
||||
|
||||
- **Step 2: Update react-client lint script**
|
||||
|
||||
In `libs/react-client/package.json`, change:
|
||||
|
||||
```diff
|
||||
- "lint": "eslint ./src --ext .ts,.tsx",
|
||||
+ "lint": "eslint",
|
||||
```
|
||||
|
||||
- **Step 3: Update copilot lint script**
|
||||
|
||||
In `libs/copilot/package.json`, change:
|
||||
|
||||
```diff
|
||||
- "lint": "eslint ./src --ext .ts,.tsx",
|
||||
+ "lint": "eslint",
|
||||
```
|
||||
|
||||
- **Step 4: Verify each package lints correctly**
|
||||
|
||||
```bash
|
||||
pnpm --filter @chainlit/app lint
|
||||
pnpm --filter @chainlit/react-client lint
|
||||
pnpm --filter @chainlit/copilot lint
|
||||
```
|
||||
|
||||
Expected: each lints its own directory (`.` = package root). No config errors.
|
||||
|
||||
> **Note:** This now also lints files outside `src/` (e.g. `vite.config.ts`, test files, storybook files). This is intentional and improves coverage. If any new lint errors surface in those files, fix them before proceeding.
|
||||
|
||||
- **Step 5: Verify root lint script still works**
|
||||
|
||||
```bash
|
||||
pnpm lint
|
||||
```
|
||||
|
||||
Expected: runs all three packages' lint in parallel, same as before.
|
||||
|
||||
- **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add frontend/package.json libs/react-client/package.json libs/copilot/package.json
|
||||
git commit -m "chore: simplify package lint scripts for eslint v9 flat config"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 5: Update lint-staged Config
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `lint-staged.config.js`
|
||||
|
||||
This is the payoff: package-scoped globs with `pnpm --filter` for ESLint. Prettier stays as direct invocation (it already uses the root `.prettierrc` and lint-staged passes staged files directly).
|
||||
|
||||
- **Step 1: Replace `lint-staged.config.js` contents**
|
||||
|
||||
```js
|
||||
export default {
|
||||
// ── Frontend ──────────────────────────────────────
|
||||
'frontend/**/*.{ts,tsx}': [
|
||||
'pnpm --filter @chainlit/app lint -- --fix',
|
||||
'prettier --write',
|
||||
() => 'pnpm --filter @chainlit/app type-check'
|
||||
],
|
||||
|
||||
// ── React Client ──────────────────────────────────
|
||||
'libs/react-client/**/*.{ts,tsx}': [
|
||||
'pnpm --filter @chainlit/react-client lint -- --fix',
|
||||
'prettier --write',
|
||||
() => 'pnpm --filter @chainlit/react-client type-check'
|
||||
],
|
||||
|
||||
// ── Copilot ───────────────────────────────────────
|
||||
'libs/copilot/**/*.{ts,tsx}': [
|
||||
'pnpm --filter @chainlit/copilot lint -- --fix',
|
||||
'prettier --write',
|
||||
() => 'pnpm --filter @chainlit/copilot type-check'
|
||||
],
|
||||
|
||||
// ── Cypress (not a workspace package) ─────────────
|
||||
'cypress/**/*.ts': ['eslint --fix', 'prettier --write'],
|
||||
|
||||
// ── Root config files ─────────────────────────────
|
||||
'*.{js,cjs,mjs}': ['eslint --fix', 'prettier --write'],
|
||||
|
||||
// ── Python ────────────────────────────────────────
|
||||
'backend/**/*.py': [
|
||||
'uv run ruff check',
|
||||
'uv run ruff format --check',
|
||||
() => 'uv run dmypy run -- backend/'
|
||||
],
|
||||
|
||||
// ── GitHub Actions ────────────────────────────────
|
||||
'.github/workflows/**': ['actionlint']
|
||||
};
|
||||
```
|
||||
|
||||
> **How `pnpm --filter @chainlit/app lint -- --fix` works with lint-staged:**
|
||||
>
|
||||
> 1. lint-staged matches staged files against the glob (e.g. `frontend/**/*.{ts,tsx}`)
|
||||
> 2. For string commands, lint-staged appends matched file paths as arguments
|
||||
> 3. Command becomes: `pnpm --filter @chainlit/app lint -- --fix /abs/path/to/file1.tsx /abs/path/to/file2.tsx`
|
||||
> 4. pnpm passes everything after `--` to the package's `lint` script
|
||||
> 5. Script expands to: `eslint --fix /abs/path/to/file1.tsx /abs/path/to/file2.tsx`
|
||||
> 6. ESLint v9: explicit file args override the default `.` — lints **only** the staged files
|
||||
>
|
||||
> **Why prettier stays as direct invocation:** Prettier doesn't have a "default to `.`" mode. The root `.prettierrc` is already the single source of config. lint-staged passes staged files directly — no benefit from `pnpm --filter` indirection.
|
||||
>
|
||||
> **Why** `backend/**/*.py` **instead of** `**/*.py`: Scopes Python linting to backend only, consistent with the package-scoped approach for JS/TS.
|
||||
|
||||
- **Step 2: Verify lint-staged config is valid**
|
||||
|
||||
```bash
|
||||
npx lint-staged --debug 2>&1 | head -20
|
||||
```
|
||||
|
||||
Expected: no syntax/parse errors.
|
||||
|
||||
- **Step 3: Test with a staged file**
|
||||
|
||||
```bash
|
||||
# Stage a minor whitespace change in a frontend file, then:
|
||||
npx lint-staged --verbose
|
||||
```
|
||||
|
||||
Expected: runs `pnpm --filter @chainlit/app lint -- --fix` with only the staged file, NOT the entire package.
|
||||
|
||||
- **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add lint-staged.config.js
|
||||
git commit -m "chore: scope lint-staged to packages with pnpm --filter"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 6: Fix Lint Errors in Newly Covered Files
|
||||
|
||||
ESLint now covers files outside `src/` (config files, tests, storybook). Some may have pre-existing lint issues.
|
||||
|
||||
**Files:**
|
||||
|
||||
- Varies — depends on what ESLint reports
|
||||
- **Step 1: Run full lint to find new errors**
|
||||
|
||||
```bash
|
||||
pnpm lint 2>&1
|
||||
```
|
||||
|
||||
- **Step 2: Fix any errors**
|
||||
|
||||
Most likely candidates:
|
||||
|
||||
- `vite.config.ts` files — may use `require()` or have unused imports
|
||||
- `frontend/tests/*.spec.tsx` — may have unused variables in test helpers
|
||||
- `libs/copilot/.storybook/*.ts` — may have `any` types (but rule is off)
|
||||
- `libs/copilot/stories/*.ts` — storybook conventions
|
||||
|
||||
Fix each file. If a file genuinely shouldn't be linted (e.g. auto-generated), add it to the `ignores` array in `eslint.config.mjs`.
|
||||
|
||||
- **Step 3: Verify clean lint**
|
||||
|
||||
```bash
|
||||
pnpm lint
|
||||
```
|
||||
|
||||
Expected: exit 0, no errors.
|
||||
|
||||
- **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add -A
|
||||
git commit -m "fix: resolve lint errors in newly covered files"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 7: Final Verification
|
||||
|
||||
- **Step 1: Run full CI-equivalent checks**
|
||||
|
||||
```bash
|
||||
pnpm lint && pnpm type-check && pnpm --filter @chainlit/app test
|
||||
```
|
||||
|
||||
Expected: all pass.
|
||||
|
||||
- **Step 2: Test lint-staged end-to-end**
|
||||
|
||||
Make a trivial change to a file in each package, stage it, and run:
|
||||
|
||||
```bash
|
||||
npx lint-staged --verbose
|
||||
```
|
||||
|
||||
Verify:
|
||||
|
||||
- Frontend file change triggers only `@chainlit/app` lint + type-check
|
||||
- react-client file change triggers only `@chainlit/react-client` lint + type-check
|
||||
- A Python file change triggers only ruff + dmypy
|
||||
- No cross-package lint runs
|
||||
- **Step 3: Commit any remaining changes**
|
||||
|
||||
```bash
|
||||
git add -A
|
||||
git commit -m "chore: complete eslint v9 migration and lint-staged unification"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Risk Checklist
|
||||
|
||||
| Risk | Mitigation |
|
||||
| --------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------ |
|
||||
| `eslint-plugin-chai-friendly` doesn't work in flat config | It exports `configs.recommendedFlat` (verified at runtime). If it fails, wrap with `FlatCompat` from `@eslint/eslintrc`. |
|
||||
| `eslint-plugin-cypress` v6 breaking changes | The v6 `/flat` export is documented. The plugin only adds 2 rules (`no-assigning-return-values`, `no-unnecessary-waiting`). Verify they still trigger. |
|
||||
| New lint errors in previously unlinted files | Task 6 handles this. Scope is small: ~10 files outside `src/` across all packages. |
|
||||
| `pnpm --filter` + lint-staged file passing | Verified empirically: lint-staged appends absolute paths, pnpm passes them after `--`, ESLint v9 lints only those files. |
|
||||
@@ -0,0 +1,308 @@
|
||||
# CI Cache Optimization P2 Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Build the frontend artifact that CI still duplicates today exactly once per commit and restore it in downstream jobs, cutting repeated `@chainlit/react-client` builds from the `check-frontend` matrix.
|
||||
|
||||
**Architecture:** On the current `feat/refactor-scripts` branch, P1 is already implemented and P3 has already collapsed the old `prepare`/`validate` split in `e2e-tests`. The original P2 write-up in `docs/research/ci-cache-optimization.md` assumes `pnpm run buildUi` still runs in `tests` and `e2e-tests`, but that is no longer true in the current workflows. The remaining explicit duplicated frontend build is `pnpm --filter @chainlit/react-client build` inside the three-leg `check-frontend` matrix, so this plan seeds `libs/react-client/dist` once in `ci.yaml`, restores it in `check-frontend.yaml`, and leaves `tests`, `e2e-tests`, and backend checks untouched.
|
||||
|
||||
**Tech Stack:** GitHub Actions reusable workflows, `actions/cache@v5`, pnpm 9, TypeScript 5, tsup
|
||||
|
||||
## Assumptions
|
||||
|
||||
- P1 is already merged on this branch: `e2e-tests.yaml` uses `actions/cache@v5` for the Cypress binary and no longer uses `cypress-io/github-action`.
|
||||
- P3 is already merged on this branch: `e2e-tests.yaml` has a single `prepare` job and no separate `validate` job.
|
||||
- Backward compatibility matters more than theoretical optimality. Do not rename existing top-level CI jobs that branch protection may already depend on.
|
||||
- The cache boundary for this iteration is only `libs/react-client/dist`. Do not broaden scope to `frontend/dist` or `libs/copilot/dist` unless a fresh measurement proves they are still rebuilt in CI.
|
||||
|
||||
## Success Criteria
|
||||
|
||||
- `check-frontend` no longer runs `pnpm --filter @chainlit/react-client build` in each matrix job.
|
||||
- A new cache-seeding job restores or builds `libs/react-client/dist` once per `github.sha`.
|
||||
- `check-frontend` fails fast if the expected cache is missing, instead of silently rebuilding.
|
||||
- `check-backend`, `tests`, and `e2e-tests` keep their current behavior.
|
||||
- A second CI run for the same commit shows a cache hit in the seed job and no rebuild.
|
||||
|
||||
---
|
||||
|
||||
## File Map
|
||||
|
||||
| Action | File | Responsibility |
|
||||
| ------ | ---------------------------------------- | ---------------------------------------------------------------------------------- |
|
||||
| Modify | `.github/workflows/ci.yaml` | Add a one-time cache-seeding job and wire existing jobs to depend on it |
|
||||
| Modify | `.github/workflows/check-frontend.yaml` | Restore `libs/react-client/dist` from cache and remove the duplicated inline build |
|
||||
| Modify | `docs/research/ci-cache-optimization.md` | Update P2 notes so the research doc matches the current branch reality |
|
||||
|
||||
---
|
||||
|
||||
## Task 1: Seed `@chainlit/react-client` Build Cache Once
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `.github/workflows/ci.yaml`
|
||||
|
||||
- [ ] **Step 1: Add a dedicated cache-seeding job**
|
||||
|
||||
Insert this job above `check-frontend` in `.github/workflows/ci.yaml`:
|
||||
|
||||
```yaml
|
||||
build-react-client:
|
||||
name: Build @chainlit/react-client once
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v6
|
||||
- uses: ./.github/actions/pnpm-node-install
|
||||
name: Install Node, pnpm and dependencies.
|
||||
- name: Cache @chainlit/react-client dist
|
||||
id: react-client-dist
|
||||
uses: actions/cache@v5
|
||||
with:
|
||||
path: libs/react-client/dist
|
||||
key: react-client-build-${{ github.sha }}
|
||||
- name: Build @chainlit/react-client
|
||||
if: steps.react-client-dist.outputs.cache-hit != 'true'
|
||||
run: pnpm --filter @chainlit/react-client build
|
||||
```
|
||||
|
||||
Why this exact shape:
|
||||
|
||||
- `github.sha` is the safest cache key for compiled output because it matches the exact source tree used by downstream jobs.
|
||||
- `actions/cache@v5` restores immediately and saves at job completion, so downstream jobs can rely on the cache after `needs: build-react-client`.
|
||||
- `ubuntu-latest` matches the current `check-frontend` runner, so no cross-OS cache settings are needed.
|
||||
|
||||
- [ ] **Step 2: Wire `check-frontend` and the CI aggregator to the new job**
|
||||
|
||||
In `.github/workflows/ci.yaml`, change the existing jobs to:
|
||||
|
||||
```yaml
|
||||
check-frontend:
|
||||
needs: [build-react-client]
|
||||
uses: ./.github/workflows/check-frontend.yaml
|
||||
secrets: inherit
|
||||
|
||||
ci:
|
||||
runs-on: ubuntu-slim
|
||||
name: Run CI
|
||||
if: always()
|
||||
needs: [check-backend, build-react-client, check-frontend, tests, e2e-tests]
|
||||
```
|
||||
|
||||
The extra `build-react-client` entry in `ci.needs` is required. Without it, a failed seed job can cause `check-frontend` to be skipped and the final `ci` job would not treat that upstream failure as a hard failure.
|
||||
|
||||
- [ ] **Step 3: Verify the workflow file still formats cleanly**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
pnpm format-check:files .github/workflows/ci.yaml
|
||||
```
|
||||
|
||||
Expected: exit code `0` and no output complaining about YAML formatting.
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add .github/workflows/ci.yaml
|
||||
git commit -m "ci: seed react-client build cache once per commit"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 2: Restore the Cache in `check-frontend`
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `.github/workflows/check-frontend.yaml`
|
||||
|
||||
- [ ] **Step 1: Replace the inline build step with cache restore + explicit failure**
|
||||
|
||||
In `.github/workflows/check-frontend.yaml`, replace:
|
||||
|
||||
```yaml
|
||||
- name: Build @chainlit/react-client
|
||||
run: pnpm --filter @chainlit/react-client build
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```yaml
|
||||
- name: Restore @chainlit/react-client dist
|
||||
id: react-client-dist
|
||||
uses: actions/cache@v5
|
||||
with:
|
||||
path: libs/react-client/dist
|
||||
key: react-client-build-${{ github.sha }}
|
||||
|
||||
- name: Fail if @chainlit/react-client dist cache is missing
|
||||
if: steps.react-client-dist.outputs.cache-hit != 'true'
|
||||
run: |
|
||||
echo "❌ Error: expected react-client build cache for ${GITHUB_SHA}"
|
||||
exit 1
|
||||
```
|
||||
|
||||
Do **not** add a fallback rebuild here. The point of P2 is to prove the seed job is the single source of truth for this build artifact. Silent rebuilds would hide broken cache wiring.
|
||||
|
||||
- [ ] **Step 2: Keep the matrix commands unchanged**
|
||||
|
||||
After the cache restore, the job should still run the existing commands exactly as they are today:
|
||||
|
||||
```yaml
|
||||
- name: ${{ matrix.name }}
|
||||
run: ${{ matrix.command }}
|
||||
```
|
||||
|
||||
This preserves the existing job names:
|
||||
|
||||
- `Linting: frontend`
|
||||
- `Formatting: frontend`
|
||||
- `Type checking: frontend`
|
||||
|
||||
and avoids unnecessary branch-protection churn.
|
||||
|
||||
- [ ] **Step 3: Run the local smoke checks in the same order CI depends on**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
pnpm --filter @chainlit/react-client build
|
||||
pnpm lint
|
||||
pnpm format-check
|
||||
pnpm type-check
|
||||
```
|
||||
|
||||
Expected:
|
||||
|
||||
- `pnpm --filter @chainlit/react-client build` writes `libs/react-client/dist`
|
||||
- `pnpm lint` passes
|
||||
- `pnpm format-check` passes
|
||||
- `pnpm type-check` passes
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add .github/workflows/check-frontend.yaml
|
||||
git commit -m "ci: restore react-client build output in frontend checks"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 3: Update the Research Doc So P2 Is No Longer Stale
|
||||
|
||||
**Files:**
|
||||
|
||||
- Modify: `docs/research/ci-cache-optimization.md`
|
||||
|
||||
- [ ] **Step 1: Rewrite the P2 problem statement to match the current branch**
|
||||
|
||||
Update the P2 section so it says the following, in substance:
|
||||
|
||||
```md
|
||||
### P2: Build `@chainlit/react-client` once at CI startup, share via cache
|
||||
|
||||
**Status**: Still useful on `feat/refactor-scripts`, but the scope is narrower than the original draft.
|
||||
|
||||
**Current reality on this branch**:
|
||||
|
||||
- `tests.yaml` no longer runs `pnpm run buildUi`
|
||||
- `e2e-tests.yaml` no longer runs `pnpm run buildUi`
|
||||
- `check-frontend.yaml` still runs `pnpm --filter @chainlit/react-client build` once per matrix leg
|
||||
|
||||
**Approach**: Seed `libs/react-client/dist` once in `ci.yaml` with `actions/cache`, then restore it in `check-frontend.yaml` and fail on cache miss instead of rebuilding.
|
||||
```
|
||||
|
||||
Keep the rest of the research document intact. This is a surgical doc correction, not a fresh rewrite.
|
||||
|
||||
- [ ] **Step 2: Update the expected impact table**
|
||||
|
||||
Change the P2 estimate from the old “buildUi in 7 jobs” numbers to the narrower current branch estimate:
|
||||
|
||||
```md
|
||||
| Change | Current | After | Saving |
|
||||
| ------------------------------------------------------------ | ---------------------- | ----------- | ----------------------------- |
|
||||
| P2: cache `@chainlit/react-client` build in `check-frontend` | ~20-40s per matrix leg | ~5s restore | ~45-90s cumulative per CI run |
|
||||
```
|
||||
|
||||
Use the final measured numbers from the first successful CI run if they differ materially from this estimate.
|
||||
|
||||
- [ ] **Step 3: Verify markdown formatting**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
pnpm format-check:files docs/research/ci-cache-optimization.md
|
||||
```
|
||||
|
||||
Expected: exit code `0`.
|
||||
|
||||
- [ ] **Step 4: Commit**
|
||||
|
||||
```bash
|
||||
git add docs/research/ci-cache-optimization.md
|
||||
git commit -m "docs: narrow P2 cache optimization scope to current workflows"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Task 4: Verify the New Workflow Behavior in CI
|
||||
|
||||
**Files:**
|
||||
|
||||
- No file changes
|
||||
|
||||
- [ ] **Step 1: Push the branch and wait for the `CI` workflow to finish**
|
||||
|
||||
Run:
|
||||
|
||||
```bash
|
||||
git push
|
||||
```
|
||||
|
||||
Expected: a new `CI` workflow run starts for the current commit.
|
||||
|
||||
- [ ] **Step 2: Inspect the first run**
|
||||
|
||||
Verify in the GitHub Actions UI:
|
||||
|
||||
- `build-react-client` shows either:
|
||||
- cache miss + one `pnpm --filter @chainlit/react-client build`, or
|
||||
- cache hit + skipped build on a rerun of the same commit
|
||||
- each `check-frontend` matrix job restores `react-client-build-${{ github.sha }}`
|
||||
- no `check-frontend` matrix job logs an inline `pnpm --filter @chainlit/react-client build`
|
||||
|
||||
- [ ] **Step 3: Re-run the same commit once**
|
||||
|
||||
Use the GitHub Actions UI to re-run the workflow for the same SHA.
|
||||
|
||||
Expected:
|
||||
|
||||
- `build-react-client` shows `cache-hit: true`
|
||||
- the build step is skipped
|
||||
- all `check-frontend` matrix jobs still restore the cache and pass
|
||||
|
||||
- [ ] **Step 4: Record the measured timings in the research doc**
|
||||
|
||||
If the measured timing differs from the estimate in Task 3, update the numbers in `docs/research/ci-cache-optimization.md` before merging.
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add docs/research/ci-cache-optimization.md
|
||||
git commit -m "docs: record measured P2 CI cache results"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- Do not reintroduce a full `buildUi` job unless fresh evidence shows `tests` or `e2e-tests` need it again.
|
||||
- Do not cache `frontend/dist` or `libs/copilot/dist` in this change.
|
||||
- Do not change `pnpm-node-install` or `uv-python-install`; P2 does not require action-level abstraction.
|
||||
- Do not change publish workflows as part of this task.
|
||||
|
||||
## Self-Review
|
||||
|
||||
**Spec coverage:** This plan covers P2 only and explicitly adapts it to the current branch state after P1 and P3. It does not assume the stale “buildUi in 7 jobs” model from the research draft.
|
||||
|
||||
**Placeholder scan:** No `TODO`, `TBD`, or “handle appropriately” placeholders remain. Each workflow edit has exact YAML and each verification step has an exact command or a concrete UI check.
|
||||
|
||||
**Consistency:** The cache key is consistently `react-client-build-${{ github.sha }}` and the cached path is consistently `libs/react-client/dist` in both producer and consumer jobs.
|
||||
Reference in New Issue
Block a user