diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 1574efbcd1..1b43ce895d 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -43,7 +43,12 @@ Project maintainers have the right and responsibility to remove, edit, or reject ## 2. Development Notes -1. Before starting work, make sure your main branch has the latest commits with `npm run update`. +1. Before starting work, sync `dev` from this repository. You are working in a fork, so `origin` is + your fork — add the canonical remote once and sync from it: + - `git remote add upstream https://github.com/danny-avila/LibreChat.git` + - `git fetch upstream dev && git checkout -B dev upstream/dev` + - `npm run update` is the self-host deployment updater — it checks out `main` and rebuilds your + containers. Do not use it to refresh a development branch. 2. Run linting command to find errors: `npm run lint`. Alternatively, ensure husky pre-commit checks are functioning. - `npm install` sets the hooks up for you; set `HUSKY=0` to opt out. - The pre-commit hook runs the Static Checks CI job locally, scoped to the files in the commit. Run it by hand with `npm run static-checks`, against a base ref with `npm run static-checks -- --against origin/dev`, or with the slow gates (TypeScript, config migration tests, unused i18n keys, unused npm packages) via `npm run static-checks:full`. @@ -60,11 +65,11 @@ Project maintainers have the right and responsibility to remove, edit, or reject We utilize a GitFlow workflow to manage changes to this project's codebase. Follow these general steps when contributing code: -1. Fork the repository and create a new branch with a descriptive slash-based name (e.g., `new/feature/x`). +1. Fork the repository and branch off `dev` with a descriptive slash-based name (e.g., `new/feature/x`). All contributions target `dev`; `main` only moves at release time, and pull requests opened against it are retargeted automatically. 2. Implement your changes and ensure that all tests pass. 3. Commit your changes using conventional commit messages with GitFlow flags. Begin the commit message with a tag indicating the change type, such as "feat" (new feature), "fix" (bug fix), "docs" (documentation), or "refactor" (code refactoring), followed by a brief summary of the changes (e.g., `feat: Add new feature X to the project`). -4. Submit a pull request with a clear and concise description of your changes and the reasons behind them. -5. We will review your pull request, provide feedback as needed, and eventually merge the approved changes into the main branch. +4. Submit a pull request against `dev` with a clear and concise description of your changes and the reasons behind them. +5. We will review your pull request, provide feedback as needed, and eventually merge the approved changes into the `dev` branch. ## 4. Commit Message Format diff --git a/.github/scripts/retarget-prs.sh b/.github/scripts/retarget-prs.sh new file mode 100755 index 0000000000..f9358b4efc --- /dev/null +++ b/.github/scripts/retarget-prs.sh @@ -0,0 +1,155 @@ +#!/usr/bin/env bash +# Retarget pull requests opened against the release branch onto the development branch. +# Used by .github/workflows/pr-retarget-dev.yml for both the on-open hook and the manual sweep. +set -euo pipefail + +REPO="${REPO:?REPO is required (owner/name)}" +RELEASE_BASE="${RELEASE_BASE:-main}" +TARGET_BASE="${TARGET_BASE:-dev}" +DRY_RUN="${DRY_RUN:-false}" +EXPLAIN_MISSING="${EXPLAIN_MISSING:-false}" +KEEP_LABEL="${KEEP_LABEL:-target: main}" +THROTTLE_SECONDS="${THROTTLE_SECONDS:-0}" +MARKER="" + +if [ "$#" -eq 0 ]; then + echo "usage: REPO=owner/name $0 [pr-number...]" >&2 + exit 64 +fi + +# Branches on the upstream repository that legitimately merge into the release branch. +# Backport branches are deliberately absent: `main` is kept as a fast-forward of `dev`, so a +# backport merged straight to `main` would break that invariant. Use the label to exempt one. +keeps_release_base() { + local head_repo="$1" head_ref="$2" + [ "$head_repo" = "$REPO" ] || return 1 + case "$head_ref" in + "$TARGET_BASE" | release/* | hotfix/*) return 0 ;; + *) return 1 ;; + esac +} + +# 0 = already explained, 1 = not explained, 2 = the lookup itself failed. The third status +# matters: treating a failed read as "not explained" would post a duplicate. Bodies are collected +# before grepping because piping into `grep -q` lets SIGPIPE fail the pipeline under `pipefail`. +already_explained() { + local bodies + bodies="$(gh api "repos/$REPO/issues/$1/comments" --paginate --jq '.[].body')" || return 2 + grep -qF "$MARKER" <<<"$bodies" +} + +comment_body() { + cat </dev/null; then + echo "#$number: skipped — labelled '$KEEP_LABEL'" + skipped=$((skipped + 1)) + continue + fi + + if keeps_release_base "$head_repo" "$head_ref"; then + echo "#$number: skipped — $head_repo:$head_ref is a release-bound branch" + skipped=$((skipped + 1)) + continue + fi + + if [ "$DRY_RUN" = "true" ]; then + echo "#$number: would retarget $RELEASE_BASE -> $TARGET_BASE ($head_repo:$head_ref)" + matched=$((matched + 1)) + continue + fi + + echo "#$number: retargeting $RELEASE_BASE -> $TARGET_BASE ($head_repo:$head_ref)" + if ! gh pr edit "$number" --repo "$REPO" --base "$TARGET_BASE"; then + echo "#$number: FAILED to change base branch" + failed=$((failed + 1)) + continue + fi + post_explanation "$number" + matched=$((matched + 1)) + [ "$THROTTLE_SECONDS" = "0" ] || sleep "$THROTTLE_SECONDS" +done + +if [ "$DRY_RUN" = "true" ]; then + echo "DRY RUN — no pull request was modified. would_retarget=$matched skipped=$skipped failed=$failed" + summary="**Dry run — nothing was modified.** Would retarget **$matched**, skip **$skipped**, failed to read **$failed**." +else + echo "retargeted=$matched skipped=$skipped failed=$failed unexplained=$unexplained" + summary="Retargeted **$matched** pull request(s) onto \`$TARGET_BASE\`, skipped **$skipped**, failed **$failed**, missing an explanation **$unexplained**." +fi + +echo "$summary" +[ -z "${GITHUB_STEP_SUMMARY:-}" ] || echo "$summary" >> "$GITHUB_STEP_SUMMARY" + +[ "$failed" -eq 0 ] && [ "$unexplained" -eq 0 ] diff --git a/.github/workflows/pr-retarget-dev.yml b/.github/workflows/pr-retarget-dev.yml new file mode 100644 index 0000000000..da4cf02638 --- /dev/null +++ b/.github/workflows/pr-retarget-dev.yml @@ -0,0 +1,79 @@ +name: Retarget PRs to dev + +on: + pull_request_target: + types: [opened, reopened, synchronize] + branches: [main] + workflow_dispatch: + inputs: + dry_run: + description: 'Report what would change without editing any pull request' + type: boolean + default: true + pr_numbers: + description: 'Space-separated PR numbers (default: every open pull request based on main)' + required: false + default: '' + +permissions: + contents: read + pull-requests: write + +# Per-pull-request for the hook so a burst of openings runs in parallel; GitHub keeps only one +# pending job per group, so a single shared group would cancel queued retargets. Every sweep shares +# one group so two of them cannot process the same pull request at once. +concurrency: + group: pr-retarget-dev-${{ github.event.pull_request.number || 'sweep' }} + cancel-in-progress: false + +jobs: + on-open: + name: Retarget on open + if: github.event_name == 'pull_request_target' + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v5 + with: + # pull_request_target runs with a write token: check out the trusted base + # commit only, never the pull request head. + ref: ${{ github.event.pull_request.base.sha }} + persist-credentials: false + sparse-checkout: .github/scripts + + - name: Retarget onto dev + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + run: .github/scripts/retarget-prs.sh "$PR_NUMBER" + + sweep: + name: Sweep open pull requests + if: github.event_name == 'workflow_dispatch' + runs-on: ubuntu-latest + timeout-minutes: 60 + steps: + - uses: actions/checkout@v5 + with: + persist-credentials: false + sparse-checkout: .github/scripts + + - name: Retarget onto dev + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + REPO: ${{ github.repository }} + DRY_RUN: ${{ inputs.dry_run }} + PR_NUMBERS: ${{ inputs.pr_numbers }} + THROTTLE_SECONDS: '2' + run: | + numbers="$PR_NUMBERS" + if [ -z "$numbers" ]; then + numbers="$(gh api --paginate "repos/$REPO/pulls?base=main&state=open&per_page=100" --jq '.[].number')" + fi + if [ -z "$numbers" ]; then + echo "No open pull requests based on main." + exit 0 + fi + # shellcheck disable=SC2086 + .github/scripts/retarget-prs.sh $numbers diff --git a/AGENTS.md b/AGENTS.md index 287d5357f2..13a1570a26 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,5 +1,23 @@ See CLAUDE.md. +## Branching and pull requests + +Branch off `dev` and target `dev` with every pull request; `gh pr create` defaults to `main`, so +pass `--base dev` explicitly. `main` is the released branch, kept as a fast-forward of `dev` and +synced as-is — never open a backport pull request to `main`, because anything merged to `dev` +reaches it at the next sync. Pull requests opened against `main` are retargeted automatically. +`Fixes #N` does not close the issue on a `dev` merge — GitHub honors closing keywords only on the +default branch, so close linked issues by hand. Worktrees share one stash stack, so never use a bare +`git stash pop`. See the detailed policy in `CLAUDE.md` under "Branching and Pull Requests". + +## Verification + +A green build is not a typecheck: `packages/api`, `packages/client` and `packages/data-schemas` build +with `tsdown`, which emits without checking types. Run `npx tsc --noEmit` in the workspace you +changed. `packages/client` excludes `*.spec.ts(x)` and `*.test.ts(x)` from typechecking entirely. +`npm run sort-imports` with no arguments rewrites every source root — pass the paths you touched. See +`CLAUDE.md` under "Typechecking" and "Formatting". + ## Frontend theming and styling For frontend work, compose existing `@librechat/client` primitives and variants before adding diff --git a/CLAUDE.md b/CLAUDE.md index bff3a7ebd1..7594b29027 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,6 +28,29 @@ The source code for `@librechat/agents` (major backend dependency, same team) li --- +## Branching and Pull Requests + +- **Branch off `dev`, and target `dev` with every pull request.** All work lands on `dev` first. +- **`main` is the released branch.** It is kept as a fast-forward of `dev` and synced as-is, so it + is always an ancestor of `dev` — equal to it right after a sync, behind it otherwise. It never + carries a commit that `dev` does not have. +- **Never open a backport pull request to `main`.** Anything merged to `dev` reaches `main` at the + next sync; a second pull request for the same change is redundant. +- **The repository's default branch is `main`**, so `gh pr create` and the GitHub UI target it + unless told otherwise — always pass `--base dev` explicitly. +- Pull requests opened against `main` are retargeted to `dev` automatically by + `.github/workflows/pr-retarget-dev.yml`. The `target: main` label exempts one, as do release-bound + upstream branches (`dev`, `release/*`, `hotfix/*`). Backport branches are deliberately not exempt — + a backport merged straight to `main` is what breaks the fast-forward invariant. +- **`Fixes #N` does not close the issue.** GitHub honors closing keywords only when a pull request + merges into the default branch (`main`). Merging to `dev` does not close anything, and the later + fast-forward of `main` is not a merge event either — close linked issues by hand. +- **Git worktrees share one stash stack.** `refs/stash` lives in the common `.git` directory, so a + bare `git stash pop` in one worktree can take work stashed in another. Prefer a throwaway WIP + commit; if you must stash, `git stash push -m ` and `apply` that specific entry. + +--- + ## Code Style ### Naming and File Organization @@ -207,6 +230,20 @@ Without it, OpenID JWT request burst caching can serve a stale `req.user` until - Frontend tests: `__tests__` directories alongside components; use `test/layout-test-utils` for rendering. - Cover loading, success, and error states for UI/data flows. +### Typechecking + +- **A green build is not a typecheck.** `packages/api`, `packages/client` and `packages/data-schemas` + build with `tsdown` alone, which emits without checking types. Only `packages/data-provider` runs + `tsc` as part of its build. +- Run `npx tsc --noEmit` in the workspace you changed before calling it done. `client` also exposes + it as `npm run typecheck`. +- `packages/client/tsconfig.json` excludes `*.spec.ts(x)` and `*.test.ts(x)`, so test files there are + never typechecked — a type error in a spec surfaces only when the test runs. +- `npm run static-checks` runs the Static Checks CI job locally against your staged files; + `npm run static-checks -- --against origin/dev` reproduces what CI sees for a pull request, and + `npm run static-checks:full` adds the slow gates (TypeScript, config migration tests, unused i18n + keys, unused npm packages). + ### Philosophy - **Real logic over mocks.** Exercise actual code paths with real dependencies. Mocking is a last resort. @@ -221,3 +258,7 @@ Without it, OpenID JWT request burst caching can serve a stale `req.user` until ## Formatting Fix all formatting lint errors (trailing spaces, tabs, newlines, indentation) using auto-fix when available. All TypeScript/ESLint warnings and errors **must** be resolved. + +`npm run sort-imports` with no arguments rewrites every file under `api/`, `client/src` and the four +`packages/*/src` roots — far beyond what you touched. Always pass explicit paths: +`npm run sort-imports -- path/to/file.ts`.