diff --git a/AGENTS.md b/AGENTS.md index 75836f4..237414e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,248 +1,38 @@ # AI Agent Playbook -Rules any code-generation agent must follow when editing this repository. -Keep changes minimal, validated, and consistent with existing patterns. - -## How to work in this repo - -- Prefer surgical edits. Don’t reformat unrelated code. Preserve public APIs unless required. -- Source of truth is the code: read actual files, signatures, and call sites before changing anything. -- Think before acting: understand root causes; don’t treat symptoms. - -### Stack constraints (do not drift) - -- Ruby 3.4 + Rails 8.1 -- No-build frontend: Importmap + tailwindcss-rails (Tailwind v4) -- Use `bin/dev` for local dev (Rails + Tailwind watcher + GoodJob) -- Do not introduce Node/Yarn or JS bundlers - -### Commands (run before finishing) - -- Tests: `bundle exec rails test` (all) or targeted files -- System tests: `bundle exec rails test:system` -- Coverage: `COVERAGE=1 bundle exec rails test` -- Lint: `bin/rubocop` -- ERB lint: `bundle exec erb_lint --lint-all` -- Assets: `RAILS_ENV=production bin/rails assets:precompile` -- Data: `bin/rails data:plans:seed` - -## Boundaries - -### Always - -- Keep diffs surgical; don’t shuffle files or reformat unrelated code. -- Add/update minimal tests when behavior changes. -- Use i18n keys for user-facing copy (no hardcoded UI strings). -- Keep WebMock enabled in tests; do not allow live HTTP. - -### Ask first (high-risk) - -- New dependencies (gems / JS packages / tooling). -- Database migrations or schema changes. -- Changes to provider contracts, auth/authorization, billing, or security boundaries. -- Editing Rails credentials / secrets handling. - -### Never - -- Commit secrets/credentials, master keys, or decrypted values. -- Add Node/Yarn/bundlers to the stack. -- Add debug prints/breakpoints in app/runtime code (`puts`, `pp`, `binding.pry`). - - Exception: intentional CLI output in Rake tasks is acceptable. - -## Git commits - -- Use Conventional Commits. -- Subject line ≤ 80 chars, imperative, no trailing period. -- Body (when non-trivial): explain **why** and **how** (not what). -- Types: `feat`, `fix`, `refactor`, `chore`, `test`, `docs`. - -## Project map (what goes where) - -- `app/`: Rails MVC, jobs, services (legacy), views. -- `lib/`: External providers and infra (`lib/providers`, `lib/faraday`, `lib/rack`). - - Data in `lib/data/` (e.g., `plans.yml`). Rake tasks in `lib/tasks/`. -- `lib/providers`: Provider interfaces/results + implementations. Use `Providers::Page`/`Providers::Review` at boundaries. - - Webhook resources via `Providers::Resources` (`:page`, `:review`). -- `test/`: Minitest (`integration/`, `system/`, etc.) - - VCR cassettes: `test/support/cassettes/` - - Webhook captures: `test/support/webhook_captures/` -- Controllers: root under `app/controllers/`; namespaced in matching folders. - - Base classes: `PublicBaseController`, `DashboardBaseController`, `Admin::BaseController`. - -## Decision trees (use these defaults) - -### 1) You’re touching an Actor/service object - -- Is the change trivial (typo, small conditional, tiny refactor)? - - Yes → keep the existing Actor/service shape. - - No → convert the touched service to a PORO as part of the change. -- When converting: - - Prefer explicit constructors + methods (`ThingCreator.new(...).create`) - - Or move behavior onto the relevant model when it naturally belongs there. - -### 2) You’re touching a ViewComponent / adding shared UI - -- Are you adding new UI/shared UI? - - Yes → use Rails partials under `app/views/shared/` (layout-aware; see below). -- Are you touching an existing ViewComponent? - - Small change → keep it as-is. - - Meaningful change → prefer migrating it to a partial as part of the change. - -## Rails conventions - -- Routing: keep routes simple and close to Rails defaults. - - Avoid redundant `defaults:` / `controller:` overrides when convention can solve it. - - Avoid `:as` overrides unless strictly necessary. - - Prefer `namespace` + `resources`. - - Prefer REST/CRUD: introduce a resource instead of custom actions. - -## Code style (scoped, Fizzy-inspired) - -Directional rules. Apply them to **new code** and **methods/files you already touch**. -Do not do churn-only refactors solely to “match style”. - -- Prefer expanded conditionals over guard clauses. - - Exception: early return at the very start of a method when the main body is non-trivial. -- Method ordering in classes: - 1) class methods - 2) public methods (with `initialize` first) - 3) private methods -- Order methods vertically by invocation order (top-down flow). -- Only use `!` when there is a corresponding non-`!` method. -- No blank line after visibility modifiers; indent methods under them. -- Thin controllers invoking rich domain APIs; avoid introducing “service artifacts” as glue. -- Jobs should be shallow: enqueue via `*_later`, execute logic in `*_now` / domain methods. - -## Fail fast (avoid defensive programming) - -We prefer code to break loudly when something unexpected happens. - -- Avoid “defensive” checks that mask errors: - - `respond_to?`, `try`, dynamic `send` to avoid using the real API - - broad `rescue` that swallows exceptions - - returning nil/false as a fallback for unexpected states -- Prefer explicit contracts: - - `find_by!` instead of `find_by` when it must exist - - `Hash#fetch` when a key must exist -- Rescue only specific exceptions you truly handle, and keep handling intentional. - -## Architecture direction - -### Services (Actor gem) → POROs - -- Current state: there are service objects using the Actor gem. -- Direction: migrate toward POROs inline with Rails/DHH style. -- Do not add new Actor-based services. -- Use the decision tree above to decide when to migrate. - -### Views: ViewComponents → Partials - -- Do not add new ViewComponents. -- Prefer Rails partials under `app/views/shared/`. -- Because we have 3 layouts (`public`, `application`, `admin`), prefer layout-aware shared folders: - - `app/views/shared/public/…` - - `app/views/shared/application/…` - - `app/views/shared/admin/…` -- Pass data via `locals`; avoid relying on instance vars in shared partials. - -## Coding standards & optimization - -- Less is more: prefer less code without sacrificing readability. -- Use clear, intention-revealing names; avoid vague names like `Manager`, `Handler`. -- Performance: - - Avoid N+1 (`includes`, `preload`). - - Use DB constraints/indexes for new query patterns. - - Batch processing for large datasets (`find_each`, `insert_all`). -- Dependencies: - - Do not add new dependencies for trivial tasks. - - Prefer stdlib / Rails built-ins already in the stack. - -## UI & Frontend - -- Tailwind v4 tokens live in `app/assets/tailwind/application.css`; use theme variables and brand classes. -- Forms baseline: “Labels on left” layout. -- Stimulus-first: - - Prefer controllers from `tailwindcss-stimulus-components` and `@stimulus-components/*`. - - New controllers must be generic, reusable, under `app/javascript/controllers/`, and registered in `controllers/index.js`. -- Copy must follow `docs/brand.md`. Marketing pages must follow `docs/marketing/style-guide.md`. -- No inline styles (`style="..."`). Use Tailwind utilities. -- Prefer semantic HTML and accessibility basics (labels, autocomplete where appropriate). - -## Mailers - -- Mailer views live under `app/views/mailers/`. -- Shared layout: `app/views/layouts/mailer.html.erb` -- Prefer partials under `app/views/mailers/` for shared sections. -- Use `premailer-rails` to inline styles. -- Set `deliver_later_queue_name` to `:latency_5m` in mailers. - -## Testing (must follow) - -### What tests should do - -- Tests must cover application behavior, not framework behavior. -- Tests must validate observable outcomes (rendered content, redirects, DB changes, enqueued jobs, outbound payloads). -- Avoid: - - tautological tests (re-implementing method logic in the test) - - “Rails works” tests - - status-only assertions with no behavioral check - - heavy stubbing that can’t catch regressions - -### Practical rules - -- Framework: Minitest. Fixtures only (factories have been removed). Do not add factories. -- Anatomy: setup (optional); exercise; assertions; cleanup (optional). Leave an empty line between sections. -- Controller/system tests must assert status/redirect AND content (selectors/text) or side effects. -- System tests: use `data-test-id` selectors; add `data-test-id` attributes in views when needed. -- Keep review pages small in tests (~10) for speed and deterministic assertions. - -### HTTP / VCR / webhooks - -- WebMock enabled. -- Outgoing API calls: recorded with VCR (see Project map for cassette location). - - VCR structure mirrors provider/API path segments. - - Filename: `{platform}_{resource}[optional_suffix].yml`. -- Incoming webhooks: do not VCR. Use JSON captures (see Project map). - -Canonical pattern (webhook fixture ingestion): - -```ruby -test "DataForSEO resolve with async mode" do - VCR.use_cassette("dataforseo/serp_google_maps_task_post/google_maps_page_resolve") do - result = Providers::Dataforseo::GoogleMaps::Page.new( - mode: :async, - recording: VCR.current_cassette.recording?, - platform: :google_maps - ).submit( - place_eid: @page.place_eid, - business_name: @location.name, - country: "GB" - ) - - assert_equal :scheduled, result.status - - ExternalTask.create!( - record_uuid: @page.uuid, - record_type: @page.class.name, - provider: Providers::External::DATAFORSEO, - resource: Providers::Resources::PAGE, - eid: result.eid - ) - - payload = WebhookCapture.read( - provider: Providers::External::DATAFORSEO, - platform: :google_maps, - resource: Providers::Resources::PAGE, - task_eid: result.eid - ) - assert_not_nil payload - - response = Providers::Dataforseo::GoogleMaps::Page.new( - mode: :async, - platform: :google_maps - ).ingest(payload) - - assert_equal :ok, response.status - assert_equal "ChIJrVtdwkDzdkgRHgNW25ELRtQ", response.data.place_eid - end -end +Repository-specific rules for code-generation agents. Keep changes minimal, +validated, and aligned with this gem's public API. + +## Core Workflow + +- Prefer surgical edits. Do not reformat unrelated code or shuffle files. +- Read actual files, signatures, call sites, and tests before changing code. +- Preserve public APIs unless the requested change requires a contract update. +- Keep the gem reusable; do not add host-app-specific routes, models, or copy. +- Add or update focused tests when behavior changes. +- Do not commit secrets, credentials, tokens, or decrypted values. + +## Release And Upgrade + +- Release changes must preserve existing `CHANGELOG.md` history. +- Use the release task instead of hand-editing version files and tags. +- Changelog pull request references must link to PRs, not issues. +- When a change breaks or changes a public contract, update `UPGRADE.md` in + the same change with explicit host-app migration steps. +- Before finishing release-harness changes, run the focused release tests and + a `git-cliff` smoke check. + +## Commit Messages + +- Use Conventional Commits: `feat`, `fix`, `docs`, `test`, `refactor`, or + `chore`. +- Keep the subject imperative, specific, and under 72 characters. +- Leave a blank line between the subject and body. +- Write one coherent reason per commit; split unrelated work first. +- Use the body when the reasoning matters. Explain why the change exists, + what approach was taken, and what constraints or side effects matter. +- Wrap body lines at 72 characters so commit hooks and terminal tools stay + readable. +- Avoid vague subjects such as `misc fixes`, `updates`, or `cleanup` unless + the cleanup is the actual scoped purpose. +- Mention verification in the body when it materially helps future readers. diff --git a/README.md b/README.md index e09d780..8da3e70 100644 --- a/README.md +++ b/README.md @@ -130,7 +130,12 @@ Note: `Gemfile.lock` is intentionally not tracked to avoid conflicts across Ruby ### Git hooks -We use [lefthook](https://lefthook.dev/) with the Ruby [commitlint](https://github.com/arandilopez/commitlint) gem to enforce Conventional Commits on every commit. We also use [Standard Ruby](https://standardrb.com/) to keep code style consistent. CI validates commit messages, Standard Ruby, tests, and git-cliff changelog generation on pull requests and pushes to main/master. +We use [lefthook](https://lefthook.dev/) with the Ruby +[commitlint](https://github.com/arandilopez/commitlint) gem to enforce +Conventional Commits on every commit. We also use +[Standard Ruby](https://standardrb.com/) to keep code style consistent. CI +validates commit messages, Standard Ruby, tests, and git-cliff changelog +generation on pull requests and pushes to main/master. Run the hook installer once per clone: @@ -146,11 +151,16 @@ rake install ## Release -Releases are tag-driven and published by GitHub Actions to RubyGems. Local release commands never publish directly. +Releases are tag-driven and published by GitHub Actions to RubyGems. +Local release commands never publish directly. -Install [git-cliff](https://git-cliff.org/) locally before preparing a release. The release task regenerates `CHANGELOG.md` from Conventional Commits. +Install [git-cliff](https://git-cliff.org/) locally before preparing a +release. The release task prepends the next `CHANGELOG.md` section from +Conventional Commits. -Before preparing a release, make sure you are on `main` or `master` with a clean worktree. +Before preparing a release, make sure you are on `main` or `master` with a +clean worktree. If the release contains a breaking public-contract change, +update `UPGRADE.md` with the host-app migration steps first. Then run one of: @@ -163,12 +173,13 @@ bundle exec rake 'release:prepare[0.1.0]' The task will: -1. Regenerate `CHANGELOG.md` with `git-cliff`. +1. Prepend the next `CHANGELOG.md` section with `git-cliff`. 1. Update `lib/git/markdown/version.rb`. 1. Commit the release changes. 1. Create and push the `vX.Y.Z` tag. -The `Release` workflow then runs tests, publishes the gem to RubyGems, and creates the GitHub release from the changelog entry. +The `Release` workflow then runs tests, publishes the gem to RubyGems, +and creates the GitHub release from the changelog entry. ## Contributing diff --git a/Rakefile b/Rakefile index f59cdcf..93454e8 100644 --- a/Rakefile +++ b/Rakefile @@ -22,14 +22,23 @@ def clean_worktree? system("git diff --quiet") && system("git diff --cached --quiet") end +def release_tag(version) + "v#{version}" +end + def release_version(target) target = target.to_s.strip - raise ArgumentError, "Provide patch, minor, major, or an explicit X.Y.Z version." if target.empty? + if target.empty? + message = "Provide patch, minor, major, or an explicit X.Y.Z version." + raise ArgumentError, message + end return target if target.match?(/\A\d+\.\d+\.\d+\z/) unless VALID_RELEASE_TARGETS.include?(target) - raise ArgumentError, "Invalid release target #{target.inspect}. Use #{VALID_RELEASE_TARGETS.join(", ")} or X.Y.Z." + message = "Invalid release target #{target.inspect}. Use " \ + "#{VALID_RELEASE_TARGETS.join(", ")} or X.Y.Z." + raise ArgumentError, message end major, minor, patch = GitMarkdown::VERSION.split(".").map(&:to_i) @@ -44,6 +53,50 @@ def release_version(target) end end +def validate_release_version!(version, current) + if Gem::Version.new(version) <= Gem::Version.new(current) + message = "Release version #{version} must be newer than " \ + "current version #{current}." + raise ArgumentError, message + end + + tag = release_tag(version) + if local_release_tag_exists?(tag) + raise ArgumentError, "Release tag #{tag} already exists locally." + end + if remote_release_tag_exists?(tag) + raise ArgumentError, "Release tag #{tag} already exists on origin." + end +end + +def local_release_tag_exists?(tag) + system( + "git", + "rev-parse", + "--quiet", + "--verify", + "refs/tags/#{tag}", + out: File::NULL + ) +end + +def remote_release_tag_exists?(tag) + output = `#{remote_release_tag_command(tag)} 2>&1` + status = $? + + if status.success? + true + elsif status.exitstatus == 2 + false + else + raise "Could not check origin for #{tag}: #{output.strip}" + end +end + +def remote_release_tag_command(tag) + "git ls-remote --exit-code --tags origin refs/tags/#{tag}" +end + def update_version_file(version) File.write( VERSION_PATH, @@ -57,40 +110,71 @@ def update_version_file(version) ) end +def changelog_command(version) + [ + "git-cliff", + "-c", + "cliff.toml", + "--unreleased", + "--tag", + release_tag(version), + "--prepend", + "CHANGELOG.md" + ] +end + def update_changelog(version) - success = system("git-cliff", "-c", "cliff.toml", "--unreleased", "--tag", "v#{version}", "-o", "CHANGELOG.md") - raise "git-cliff failed. Install git-cliff and make sure cliff.toml is valid." unless success - raise "git-cliff did not update CHANGELOG.md. Ensure there are Conventional Commits since the last tag." if system("git", "diff", "--quiet", "--", "CHANGELOG.md") + success = system(*changelog_command(version)) + unless success + message = "git-cliff failed. Install git-cliff and make sure " \ + "cliff.toml is valid." + raise message + end + + if system("git", "diff", "--quiet", "--", "CHANGELOG.md") + message = "git-cliff did not update CHANGELOG.md. Ensure there are " \ + "Conventional Commits since the last tag." + raise message + end end if Rake::Task.task_defined?("release") Rake::Task["release"].clear end -desc "Publishing is handled by GitHub Actions. Use release:prepare[...] instead." +desc "Publishing is handled by CI. Use release:prepare[...] instead." task :release do - abort "Use `bundle exec rake 'release:prepare[patch]'` (or minor/major/X.Y.Z). Publishing runs in GitHub Actions after the tag is pushed." + message = "Use `bundle exec rake 'release:prepare[patch]'` " \ + "(or minor/major/X.Y.Z). Publishing runs in GitHub Actions after " \ + "the tag is pushed." + abort message end namespace :release do - desc "Prepare a release: update CHANGELOG/version, commit, tag, and push. Accepts patch, minor, major, or X.Y.Z." + desc "Prepare a release: update changelog/version, commit, tag, and push." task :prepare, [:target] do |_task, args| branch = current_branch - abort "Release must run on main or master. Current branch: #{branch.inspect}." unless %w[main master].include?(branch) + unless %w[main master].include?(branch) + message = "Release must run on main or master. Current branch: " \ + "#{branch.inspect}." + abort message + end abort "Release requires a clean working tree." unless clean_worktree? version = release_version(args[:target]) current = GitMarkdown::VERSION - abort "Release version #{version} is older than current version #{current}." if Gem::Version.new(version) < Gem::Version.new(current) + validate_release_version!(version, current) update_changelog(version) update_version_file(version) + tag = release_tag(version) + sh "git add CHANGELOG.md lib/git/markdown/version.rb" sh %(git commit -m "chore(release): prepare v#{version}") - sh %(git tag -a v#{version} -m "Release v#{version}") + sh %(git tag -a #{tag} -m "Release #{tag}") sh "git push origin #{branch}" - sh "git push origin v#{version}" + sh "git push origin #{tag}" rescue ArgumentError, RuntimeError => e abort e.message end diff --git a/UPGRADE.md b/UPGRADE.md new file mode 100644 index 0000000..3a6cb49 --- /dev/null +++ b/UPGRADE.md @@ -0,0 +1,7 @@ +# Upgrade Guide + +Use this file for host-app migration notes when a release changes public +contracts, required setup, component locals, generated assets, or runtime +behavior. + +No special upgrade notes have been published yet. diff --git a/cliff.toml b/cliff.toml index 8a40f75..e729e7d 100644 --- a/cliff.toml +++ b/cliff.toml @@ -14,6 +14,10 @@ body = """ """ trim = true +[[changelog.postprocessors]] +pattern = '\(#([0-9]+)\)' +replace = '([#${1}](https://github.com/ethos-link/git-markdown/pull/${1}))' + [git] conventional_commits = true filter_unconventional = true diff --git a/test/release_task_test.rb b/test/release_task_test.rb new file mode 100644 index 0000000..2c5b5cf --- /dev/null +++ b/test/release_task_test.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +require "test_helper" +require "rake" + +unless respond_to?(:release_version, true) + load File.expand_path("../Rakefile", __dir__) +end + +class ReleaseTaskTest < Minitest::Test + def test_release_version_increments_patch_from_current_version + major, minor, patch = GitMarkdown::VERSION.split(".").map(&:to_i) + + assert_equal "#{major}.#{minor}.#{patch + 1}", release_version("patch") + end + + def test_release_version_accepts_explicit_semantic_version + assert_equal "0.3.0", release_version("0.3.0") + end + + def test_validate_release_version_rejects_current_version + error = assert_raises(ArgumentError) do + validate_release_version!("0.2.7", "0.2.7") + end + + assert_equal( + "Release version 0.2.7 must be newer than current version 0.2.7.", + error.message + ) + end + + def test_validate_release_version_rejects_existing_local_tag + @local_release_tag_exists = true + @remote_release_tag_exists = false + + error = assert_raises(ArgumentError) do + validate_release_version!("0.2.8", "0.2.7") + end + + assert_equal "Release tag v0.2.8 already exists locally.", error.message + end + + def test_validate_release_version_rejects_existing_remote_tag + @local_release_tag_exists = false + @remote_release_tag_exists = true + + error = assert_raises(ArgumentError) do + validate_release_version!("0.2.8", "0.2.7") + end + + assert_equal "Release tag v0.2.8 already exists on origin.", error.message + end + + def test_remote_release_tag_command_asks_git_to_fail_when_no_tag_matches + assert_equal( + "git ls-remote --exit-code --tags origin refs/tags/v0.2.8", + remote_release_tag_command("v0.2.8") + ) + end + + def test_changelog_command_prepends_the_next_release + assert_equal( + [ + "git-cliff", + "-c", + "cliff.toml", + "--unreleased", + "--tag", + "v0.2.8", + "--prepend", + "CHANGELOG.md" + ], + changelog_command("0.2.8") + ) + end + + private + + def local_release_tag_exists?(_tag) + @local_release_tag_exists || false + end + + def remote_release_tag_exists?(_tag) + @remote_release_tag_exists || false + end +end