diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 61f0761b..6ec87188 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -26,7 +26,7 @@ jobs: fetch-depth: 0 - name: Setup Node - uses: actions/setup-node@v6 + uses: actions/setup-node@v7 with: node-version: 22 cache: npm @@ -103,7 +103,7 @@ jobs: fetch-depth: 0 - name: Setup Node - uses: actions/setup-node@v6 + uses: actions/setup-node@v7 with: node-version: 22 cache: npm diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d65fdbe6..19a3f966 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -39,13 +39,13 @@ jobs: # actually contain their entry points — a gem missing one installs and # resolves, then dies on require. - run: bundle exec rake build_all - - uses: actions/upload-artifact@v4 + - uses: actions/upload-artifact@v7 with: name: gems path: pkg/*.gem - name: Attach gems to the release if: startsWith(github.ref, 'refs/tags/') - uses: softprops/action-gh-release@v2 + uses: softprops/action-gh-release@v3 with: files: pkg/*.gem generate_release_notes: true diff --git a/CHANGELOG.md b/CHANGELOG.md index f2bbe35f..26dbb1e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.6.2] - 2026-09-16 + +Releases `activeagent` and `actionagent` 1.6.2 from one tag. + +Agents gain releases: a digest of everything the model is given, cut on +deploy and pinned to every trace, run and evaluation run, so a score is a +statement about a specific release and a regression is attributable to the +change that caused it. Around it, five dashboard fixes: an evaluation +created on MySQL can be run, the Tools tab reads the same `agent.tools` the +runner does, a container-valued query parameter is coerced instead of +raising, a recording's detail response no longer carries the visitor's +cookies and web storage, and the MCP endpoint answers an unsupported method with 405 +instead of the dashboard page. `sign_in_path` and `sign_out_path` are now +documented. + +Upgrading: the install generator emits a new `add_agent_releases` migration +(guarded column by column); run it. Cutting a release is +`rake action_agent:agents:release[REVISION]` in the deploy. + ### Added - **Agents have releases, and every trace, run and evaluation says which one @@ -33,6 +52,120 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 service, class *and* action, so every code-path trace registered an observed per-action twin beside the synced record and could never be pinned to its release. + +### Fixed + +- **An evaluation created on MySQL can be run.** MySQL cannot give a JSON + column a default, so an evaluation saved there without `config` read it + back as `nil`, and `compare_models` raised before the runner did anything + else. `config` and `criteria` now read as the empty value their column + default supplies on other databases. (#417) +- **The Tools tab now says which schema tools an agent is offered, and + lets you change it.** The editor listed every schema tool as enabled and + read-only whatever `agent.tools` held — *"a checkbox that cannot add or + remove the tool is a control that changes nothing"* — while evaluations, + dashboard runs and the MCP facade offered exactly what that column named. + An agent whose roster had been emptied over the API ran a suite with no + tools (1/8, `expected tool not called ×6`) under a tab reading "12 + enabled". A schema tool's row now reads the roster and is switchable, and + every schema tool the host declares has a row, off unless the roster names + it — any agent may enable any of them, and a tool switched off has to keep + its row to be switched back on. A tool the agent class declares in code is + still reported rather than selected: the class offers it, and no checkbox + could change that. +- **A container-valued query parameter no longer 500s the dashboard API.** + `page`, `per_page`, `days`, `minutes`, `limit` and `after_sequence` were + read with `to_i`, which neither an Array (`minutes[]=1&minutes[]=2`) nor a + nested object (`page[x]=1`) answers. `Api::BaseController` now coerces + them: a multi-valued parameter means its first value, a nested object falls + back to the default, and the clamps that bounded the number still apply. + `sandboxes#compare` answers a `providers` value that is not a list of + names with a 400 instead of a `NoMethodError`. +- **A session recording's `show` no longer returns the visitor's cookies and + web storage.** Every other read path redacted the handoff state, but the + detail response carried `cookies`, `session_storage` and `local_storage` + unscrubbed, both as its own key and nested inside `metadata`. Both are now + stripped; only `#handoff` returns them, to the recording's owner. (#456) + +## [1.6.1] - 2026-09-16 + +Releases `activeagent` and `actionagent` 1.6.1 from one tag. + +A patch for two defects that share a failure mode: each one turns a broken +run into a plausible-looking success rather than an error. A date filter that +matched nothing reported zero instead of raising, and an agent reported that +zero as fact; telemetry that was enabled but never instrumented wrote no +traces while every configuration signal read healthy. Neither surfaced in a +test suite, because neither produces a failure — only a confident wrong +answer and an empty table. + +No new public surface and no behaviour change for anything that was already +working, so a patch under semver. Suites that filter on a date column will +report different — correct — numbers after upgrading; read the first run as a +corrected baseline. + +### Fixed + +- **A range filter on a `SchemaTools` column no longer matches nothing and + reports zero.** `permitted_filters!` validated the column against the + allowlist but passed the value through untouched, so a range hash reached + `where` unrecognized and Rails compiled `where(due_date: {"before" => x})` + to `due_date = NULL` — a predicate that matches no row. The tool returned + `{count: 0}` with no error and the model read it as a truthful empty + answer: "0 overdue tickets" against a database holding four. Equality + filters were unaffected, which is why this went unnoticed. Comparisons are + now built through Arel with the column's own type cast, under the operators + `before`, `after`, `lt`, `lte`, `gt`, `gte`, `on_or_before` and + `on_or_after`; two bounds may be given together to express a window; and an + operator outside that set raises `UnpermittedAttribute` rather than + returning zero, consistent with how an undeclared column is already + rejected. Ranges are offered for date, datetime, time and numeric columns + only — a lexical `>` on a name column answers a question nobody asked. +- **A range filter is now discoverable.** `filter_properties` described a date + column as a bare `{type: "string", format: "date"}`, so the tool surface + could not express "before today" at all and a model asking the question + correctly still had no way to ask it. Comparable columns are now offered as + `anyOf: [scalar, range object]`, with the operator roster in the schema. +- **Telemetry enabled from a host app's initializer now installs + instrumentation.** The railtie prepended `GenerationInstrumentation` only + when `Telemetry.enabled?` was already true as railties ran — before + `config/initializers/*.rb`. An app that configures telemetry in its own + initializer, which is what the documentation shows, was therefore never + instrumented: `enabled?` answered true, `local_storage` was on, the trace + model resolved and the store lambda worked when called directly, and no + generation ever produced a span to store. `configure` now installs as well + when the resulting configuration is enabled; `instrument_telemetry!` is + idempotent, so the railtie path and the configure path cannot + double-prepend and initializer order stops mattering. + +## [1.6.0] - 2026-09-14 + +Releases `activeagent` and `actionagent` 1.6.0 from one tag. + +A minor, not a patch. The cycle that began after 1.5.2 gives an agent a +caller — `current_user`, carried from whatever authenticated the call into +every `before_action`, every tool, every delegated sub-agent, every run over +MCP and every evaluation replay — so an authorization gem has something to +decide against. Around it: schema tools defined at runtime rather than only +in a file, a generator that writes the first one, those tools served +directly over MCP, and an evaluation that calls a fabricated answer a fault +instead of grading it as an honest gap. That is new public surface in both +gems, which is a minor under semver even though 1.5.2 shipped a feature as a +patch. + +Two notes for upgrades. `tools_succeeded` is now awarded only for a tool the +scenario expected, so a suite that was quietly scoring wrong-tool runs as +partial successes will report lower — read the first run as a corrected +baseline. And `actor:` is now stripped from tool arguments and from +`params[params][actor]`: the caller is a property of the run, set once by +whatever authenticated it, and can no longer be named by the model or by a +client. + +The engine's floor on the framework (`activeagent >= 1.4`) is unchanged and +still correct: 1.6.0 satisfies it. + +### Added + - **An evaluation replay runs as the evaluation's owner.** The scenario runner handed `Agent#test_execute` no caller, so every tool a replay called ran unattributed and a host scope answered empty — the suite graded an agent @@ -116,6 +249,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **The judge reads more of a scenario's notes** — 1,500 characters rather than 300 — because a suite's notes are often its rubric and the "must not" clause tends to come last. (#433) + ### Fixed - **The caller can no longer be named by the model, or by the client.** @@ -139,20 +273,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 hash naming both `provider` and `model` is now rebuilt as it was; a bare label is still parsed. The dashboard's "re-run" of a saved selection is the path this fixes. -- **The Tools tab now says which schema tools an agent is offered, and - lets you change it.** The editor listed every schema tool as enabled and - read-only whatever `agent.tools` held — *"a checkbox that cannot add or - remove the tool is a control that changes nothing"* — while evaluations, - dashboard runs and the MCP facade offered exactly what that column named. - An agent whose roster had been emptied over the API ran a suite with no - tools (1/8, `expected tool not called ×6`) under a tab reading "12 - enabled". A schema tool's row now reads the roster and is switchable, and - every schema tool the host declares has a row, off unless the roster names - it — any agent may enable any of them, and a tool switched off has to keep - its row to be switched back on. A tool the agent class declares in code is - still reported rather than selected: the class offers it, and no checkbox - could change that. - ## [1.5.2] - 2026-09-11 Releases `activeagent` and `actionagent` 1.5.2 from one tag. diff --git a/actionagent/app/controllers/action_agent/api/agent_runs_controller.rb b/actionagent/app/controllers/action_agent/api/agent_runs_controller.rb index 805fb1d4..d30bc27e 100644 --- a/actionagent/app/controllers/action_agent/api/agent_runs_controller.rb +++ b/actionagent/app/controllers/action_agent/api/agent_runs_controller.rb @@ -33,8 +33,8 @@ def index scope = scope.where(agent_id: params[:agent_id]) if params[:agent_id].present? scope = scope.where(status: params[:status]) if params[:status].present? - page = (params[:page] || 1).to_i - per_page = (params[:per_page] || 20).to_i + page = integer_param(:page, default: 1) + per_page = integer_param(:per_page, default: 20) total = scope.count runs = scope.offset((page - 1) * per_page).limit(per_page) diff --git a/actionagent/app/controllers/action_agent/api/agents_controller.rb b/actionagent/app/controllers/action_agent/api/agents_controller.rb index 0d10d9d6..329df8c7 100644 --- a/actionagent/app/controllers/action_agent/api/agents_controller.rb +++ b/actionagent/app/controllers/action_agent/api/agents_controller.rb @@ -132,9 +132,9 @@ def restore # observed from telemetry have no AgentRun rows at all, so a runs-only # list showed them as empty while their scorecard reported real traffic. def runs - minutes = params[:minutes].presence&.then { |m| m.to_i.clamp(1, 60 * 24 * 90) } - page = (params[:page] || 1).to_i - per_page = (params[:per_page] || 20).to_i + minutes = integer_param(:minutes)&.clamp(1, 60 * 24 * 90) + page = integer_param(:page, default: 1) + per_page = integer_param(:per_page, default: 20) executions = AgentExecutions.new( agents: [ @agent ], @@ -282,7 +282,7 @@ def tool_roster # with all-zero metrics beside a card and a runs list reporting real # traffic. def analytics - days = (params[:days] || 30).to_i + days = integer_param(:days, default: 30) start_date = days.days.ago.beginning_of_day runs = @agent.agent_runs.where("created_at >= ?", start_date) diff --git a/actionagent/app/controllers/action_agent/api/analytics_controller.rb b/actionagent/app/controllers/action_agent/api/analytics_controller.rb index 1bf51b36..2f799cb5 100644 --- a/actionagent/app/controllers/action_agent/api/analytics_controller.rb +++ b/actionagent/app/controllers/action_agent/api/analytics_controller.rb @@ -5,7 +5,7 @@ module Api class AnalyticsController < BaseController # GET /api/analytics def index - days = (params[:days] || 30).to_i + days = integer_param(:days, default: 30) start_date = days.days.ago.beginning_of_day # Table names are interpolated rather than written literally: the diff --git a/actionagent/app/controllers/action_agent/api/base_controller.rb b/actionagent/app/controllers/action_agent/api/base_controller.rb index 960d6e7a..07ebd9df 100644 --- a/actionagent/app/controllers/action_agent/api/base_controller.rb +++ b/actionagent/app/controllers/action_agent/api/base_controller.rb @@ -129,6 +129,26 @@ def require_execution_enabled! render json: { error: "Agent execution is disabled on this dashboard" }, status: :forbidden end + # An integer query param. A value can arrive as a container + # (`minutes[]=1&minutes[]=2`, or `page[x]=1`), and neither Array nor + # ActionController::Parameters responds to `to_i`: reading them + # directly raised NoMethodError and turned a malformed query into a + # 500. A multi-valued param means its first value; anything else that + # is not a scalar falls back to the default. + def integer_param(name, default: nil) + raw = params[name] + raw = raw.first if raw.is_a?(Array) + return default if raw.blank? || !(raw.is_a?(String) || raw.is_a?(Numeric)) + + raw.to_s.to_i + end + + # integer_param, then clamped into [min, max]. Non-numeric input becomes + # 0 and is then clamped up to `min`. + def clamped_param(name, default:, min:, max:) + integer_param(name, default: default).clamp(min, max) + end + def not_found render json: { error: "Record not found" }, status: :not_found end diff --git a/actionagent/app/controllers/action_agent/api/interactions_controller.rb b/actionagent/app/controllers/action_agent/api/interactions_controller.rb index 17c1fec0..1bb7a479 100644 --- a/actionagent/app/controllers/action_agent/api/interactions_controller.rb +++ b/actionagent/app/controllers/action_agent/api/interactions_controller.rb @@ -16,7 +16,7 @@ class InteractionsController < BaseController # GET /api/interactions def index - limit = params.fetch(:limit, DEFAULT_LIMIT).to_i.clamp(1, 200) + limit = clamped_param(:limit, default: DEFAULT_LIMIT, min: 1, max: 200) contexts = interactions_scope .includes(:contextable) @@ -140,8 +140,8 @@ def traces_for_agent(agent_id) def window_minutes return @window_minutes if defined?(@window_minutes) - raw = params[:minutes].presence - @window_minutes = raw ? raw.to_i.clamp(1, MAX_WINDOW_MINUTES) : nil + raw = integer_param(:minutes) + @window_minutes = raw ? raw.clamp(1, MAX_WINDOW_MINUTES) : nil end def interactions_scope diff --git a/actionagent/app/controllers/action_agent/api/sandboxes_controller.rb b/actionagent/app/controllers/action_agent/api/sandboxes_controller.rb index 5600ac7f..42f4084d 100644 --- a/actionagent/app/controllers/action_agent/api/sandboxes_controller.rb +++ b/actionagent/app/controllers/action_agent/api/sandboxes_controller.rb @@ -18,11 +18,16 @@ class SandboxesController < BaseController # POST /api/sandboxes/compare # Run multiple providers in a single sandbox using parallel generation jobs def compare - providers = params[:providers] || %w[anthropic openai ollama] + providers = params[:providers].nil? ? %w[anthropic openai ollama] : params[:providers] task = params[:task] sandbox_id = params[:sandbox_id] return render json: { error: "Task required" }, status: :bad_request unless task.present? + # A bare string or a nested object is a malformed request, not a list + # of one provider: reading it as a list raised NoMethodError. + unless providers.is_a?(Array) && providers.all? { |name| name.is_a?(String) } + return render json: { error: "providers must be a list of provider names" }, status: :bad_request + end return render json: { error: "At least 2 providers required" }, status: :bad_request if providers.size < 2 # Validate providers diff --git a/actionagent/app/controllers/action_agent/api/session_recordings_controller.rb b/actionagent/app/controllers/action_agent/api/session_recordings_controller.rb index e54dcab7..c9fbf78b 100644 --- a/actionagent/app/controllers/action_agent/api/session_recordings_controller.rb +++ b/actionagent/app/controllers/action_agent/api/session_recordings_controller.rb @@ -13,6 +13,12 @@ class SessionRecordingsController < BaseController before_action :set_recording, only: [ :show, :actions, :snapshot, :export, :handoff ] + # Browser state that must never leave the server in a read response: + # the handoff state a recording carries is a copy of the visitor's + # cookies and web storage. Only #handoff returns it, to the owner, when + # they continue the session. + SENSITIVE_STATE_KEYS = %w[cookies session_storage local_storage].freeze + # GET /api/session_recordings # List recordings with optional filters def index @@ -36,8 +42,8 @@ def index end # Pagination - page = (params[:page] || 1).to_i - per_page = [ (params[:per_page] || 20).to_i, 100 ].min + page = integer_param(:page, default: 1) + per_page = [ integer_param(:per_page, default: 20), 100 ].min offset = (page - 1) * per_page total = recordings.count @@ -79,10 +85,10 @@ def actions # Support pagination for large recordings if params[:after_sequence].present? - actions = actions.where("sequence > ?", params[:after_sequence].to_i) + actions = actions.where("sequence > ?", integer_param(:after_sequence, default: 0)) end - limit = [ params[:limit]&.to_i || 100, 500 ].min + limit = [ integer_param(:limit, default: 100), 500 ].min actions = actions.limit(limit) render json: { @@ -332,7 +338,7 @@ def recording_detail(recording) created_at: recording.created_at.iso8601, updated_at: recording.updated_at.iso8601, timeline: recording.timeline, - handoff_state: recording.metadata["handoff_state"], + handoff_state: safe_handoff_state(recording.metadata["handoff_state"]), agent: recording.agent_run&.agent&.slice(:id, :name), sandbox_session: recording.sandbox_session&.summary } @@ -343,9 +349,20 @@ def first_screenshot_url(recording) action&.screenshot_url(expires_in: 1.hour) end + # Strips the browser state at the top level and inside handoff_state, + # which the model stores nested (a recording's metadata carries the + # handoff as one key), so a show response never ships a session cookie. def safe_metadata(metadata) - # Remove sensitive data from metadata - metadata.except("cookies", "session_storage", "local_storage") + safe = (metadata || {}).except(*SENSITIVE_STATE_KEYS) + return safe unless safe["handoff_state"].is_a?(Hash) + + safe.merge("handoff_state" => safe_handoff_state(safe["handoff_state"])) + end + + def safe_handoff_state(handoff_state) + return handoff_state unless handoff_state.is_a?(Hash) + + handoff_state.except(*SENSITIVE_STATE_KEYS) end def generate_visitor_id diff --git a/actionagent/app/models/action_agent/evaluation.rb b/actionagent/app/models/action_agent/evaluation.rb index febcaf2f..8533dae0 100644 --- a/actionagent/app/models/action_agent/evaluation.rb +++ b/actionagent/app/models/action_agent/evaluation.rb @@ -35,6 +35,17 @@ class Evaluation < ApplicationRecord scope :recent, -> { order(updated_at: :desc) } + # MySQL cannot give a JSON column a default, so a row inserted there + # without `criteria` or `config` reads back nil. Both readers answer with + # the empty value the column default supplies on other databases. + def criteria + super || [] + end + + def config + super || {} + end + def latest_run evaluation_runs.order(created_at: :desc).first end diff --git a/actionagent/app/services/action_agent/agent_registrar.rb b/actionagent/app/services/action_agent/agent_registrar.rb index 91203c43..82bad145 100644 --- a/actionagent/app/services/action_agent/agent_registrar.rb +++ b/actionagent/app/services/action_agent/agent_registrar.rb @@ -77,8 +77,22 @@ def owner_for_trace ActionAgent.multi_tenant? ? @trace.try(:account) : nil end + # The set an observed agent is deduplicated within. + # + # `for_owner(nil)` is `none` — correct for a multi-tenant read, where an + # unresolved tenant must see nothing. But a single-tenant dashboard has no + # owner to resolve and legitimately registers with `owner` nil (see + # #owner_for_trace), and `none` makes every dedupe lookup miss: each trace + # created another copy of the same agent, and the MAX_OBSERVED_PER_OWNER + # cap never engaged because the count it reads was always zero. Scoping to + # the whole table when there is no owner concept is what `for_owner` + # already does for a model with no owner association. + def agents_for_owner(owner) + owner.nil? && !ActionAgent.multi_tenant? ? Agent.all : Agent.for_owner(owner) + end + def find_or_create_agent(owner) - agents = Agent.for_owner(owner) + agents = agents_for_owner(owner) existing = agents.find_by( service_name: @trace.service_name, agent_class_name: agent_class, diff --git a/actionagent/app/services/action_agent/evaluation_runner_service.rb b/actionagent/app/services/action_agent/evaluation_runner_service.rb index f26784a5..c3b03767 100644 --- a/actionagent/app/services/action_agent/evaluation_runner_service.rb +++ b/actionagent/app/services/action_agent/evaluation_runner_service.rb @@ -468,15 +468,17 @@ def owner end def judge_class - provider = judge_provider - model = @evaluation.judge_model.presence - options = {} - options[:model] = model if model - options.merge!(owner_provider_options(provider)) - - @judge_class ||= Class.new(ActiveAgent::Base) do - define_singleton_method(:name) { "EvaluationJudgeAgent" } - generate_with provider, **options + @judge_class ||= begin + provider = judge_provider + model = @evaluation.judge_model.presence + options = {} + options[:model] = model if model + options.merge!(owner_provider_options(provider)) + + Class.new(ActiveAgent::Base) do + define_singleton_method(:name) { "EvaluationJudgeAgent" } + generate_with provider, **options + end end end diff --git a/actionagent/config/routes.rb b/actionagent/config/routes.rb index ba3984d7..a5c160e8 100644 --- a/actionagent/config/routes.rb +++ b/actionagent/config/routes.rb @@ -161,6 +161,9 @@ # dashboard API key rather than a session, so it sits outside the api # namespace's session-authenticated controllers. post "mcp", to: "api/mcp#create" + match "mcp", to: ->(_env) { [ 405, { "Allow" => "POST" }, [] ] }, + via: [ :get, :delete ], + constraints: ->(request) { request.delete? || !request.format.html? } # MCP Streamable HTTP (2025-03-26): a client MAY open the server-to-client # SSE stream with GET, and ends a session with DELETE. This facade offers diff --git a/actionagent/lib/action_agent/version.rb b/actionagent/lib/action_agent/version.rb index 6a4b1871..f28a295a 100644 --- a/actionagent/lib/action_agent/version.rb +++ b/actionagent/lib/action_agent/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module ActionAgent - VERSION = "1.5.2" + VERSION = "1.6.2" end diff --git a/actionagent/lib/generators/action_agent/templates/action_agent.rb.erb b/actionagent/lib/generators/action_agent/templates/action_agent.rb.erb index b5b4d415..9e34a86d 100644 --- a/actionagent/lib/generators/action_agent/templates/action_agent.rb.erb +++ b/actionagent/lib/generators/action_agent/templates/action_agent.rb.erb @@ -31,6 +31,14 @@ ActionAgent.configure do |config| # config.authentication_method = nil # config.authentication_method = nil + # + # Where a browser is sent when it asks for a dashboard page without a valid + # session; unset, it gets a minimal session-expired page. API clients get a + # bare 401 either way. The sign-out path is what the header's "Sign out" + # item POSTs to (with _method=delete); unset, the item is not shown. + # + # config.sign_in_path = "/sign_in" + # config.sign_out_path = "/sign_out" # ========================================================================== # Ingest API authentication diff --git a/actionagent/test/engine_integration_test.rb b/actionagent/test/engine_integration_test.rb index f18251ae..fe5b8025 100644 --- a/actionagent/test/engine_integration_test.rb +++ b/actionagent/test/engine_integration_test.rb @@ -115,6 +115,25 @@ def setup assert_includes response.body, "active-agent-dashboard" end + test "mcp endpoint rejects unsupported transport methods" do + get "/activeagents/mcp", headers: { "Accept" => "text/event-stream" } + + assert_response :method_not_allowed + assert_equal "POST", response.headers["Allow"] + + delete "/activeagents/mcp" + + assert_response :method_not_allowed + assert_equal "POST", response.headers["Allow"] + end + + test "mcp dashboard route still renders for browsers" do + get "/activeagents/mcp", headers: { "Accept" => "text/html" } + + assert_response :success + assert_includes response.body, "active-agent-dashboard" + end + test "dashboard refuses unauthenticated access in production when no auth is configured" do Rails.env.stub(:local?, false) do get "/activeagents/console/traces" diff --git a/actionagent/test/evaluation_runner_service_test.rb b/actionagent/test/evaluation_runner_service_test.rb index 182b3d00..573434ef 100644 --- a/actionagent/test/evaluation_runner_service_test.rb +++ b/actionagent/test/evaluation_runner_service_test.rb @@ -20,6 +20,21 @@ def parse(content) end end + test "the judge class is built once per service instance" do + ActionAgent::Agent.delete_all + agent = ActionAgent::Agent.create!(name: "Judged", provider: "openai", model: "gpt-4o-mini") + evaluation = agent.evaluations.create!(name: "Quality", judge_kind: "llm", judge_model: "gpt-4o-mini", + criteria: [ { "key" => "quality", "type" => "llm_judge" } ]) + service = ActionAgent::EvaluationRunnerService.new(evaluation) + resolved = 0 + + service.stub(:judge_provider, -> { resolved += 1; :openai }) do + assert_same service.send(:judge_class), service.send(:judge_class) + end + + assert_equal 1, resolved + end + test "malformed or non-numeric judge scores are unscorable" do [ '{"score": "0.9"}', '{"score": true}', '{"score": null}', '{"score": {}}', '{"score": 0.9oops}', '{"score": 1e999}', '{"score": NaN}', "{}", "no json here", nil ].each do |content| diff --git a/actionagent/test/evaluation_test.rb b/actionagent/test/evaluation_test.rb new file mode 100644 index 00000000..303ec37a --- /dev/null +++ b/actionagent/test/evaluation_test.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require "test_helper" + +# MySQL cannot give a JSON column a default, so an evaluation saved there +# without config or criteria reads them back as nil. The readers answer with +# the empty value the column default supplies on other databases. The test +# database honours that default, so the nil is assigned rather than persisted. +class ActionAgentEvaluationTest < ActiveSupport::TestCase + test "a nil config reads as empty and names no comparison models" do + evaluation = ActionAgent::Evaluation.new(config: nil) + + assert_equal({}, evaluation.config) + assert_equal [], evaluation.compare_models + end + + test "nil criteria read as empty" do + evaluation = ActionAgent::Evaluation.new(criteria: nil) + + assert_equal [], evaluation.criteria + assert_equal [], evaluation.llm_criteria + end +end diff --git a/actionagent/test/param_coercion_test.rb b/actionagent/test/param_coercion_test.rb new file mode 100644 index 00000000..e139fdd5 --- /dev/null +++ b/actionagent/test/param_coercion_test.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require "test_helper" + +# A query value can arrive as a container (`minutes[]=1&minutes[]=2`, or +# `page[x]=1`), and neither Array nor ActionController::Parameters responds +# to `to_i`. Reading them directly raised NoMethodError and turned a +# malformed query into a 500 on every list the dashboard paginates or +# windows. A multi-valued param means its first value; a nested object is +# malformed and floors to the default. +class ParamCoercionTest < ActionDispatch::IntegrationTest + def setup + ActionAgent::Agent.delete_all + @agent = ActionAgent::Agent.create!(name: "Support", provider: "openai", model: "gpt-4o-mini") + @agent.agent_runs.create!(input_prompt: "hi", output: "hello", status: :complete) + end + + test "agent runs coerce container-valued minutes, page and per_page" do + get "/activeagents/api/agents/#{@agent.id}/runs", params: { minutes: [ 1, 2 ], page: { x: 1 }, per_page: [ 5 ] } + + assert_response :success, response.body + body = JSON.parse(response.body) + assert_equal 1, body["runs"].length + assert_equal 5, body["meta"]["per_page"] + assert_equal 1, body["meta"]["page"] + end + + test "agent analytics coerces a container-valued days param" do + get "/activeagents/api/agents/#{@agent.id}/analytics", params: { days: [ 7, 30 ] } + + assert_response :success, response.body + end + + test "interactions coerce container-valued minutes and limit" do + context = ActionAgent::AgentContext.create!(contextable: @agent, agent_name: "SupportAgent", action_name: "respond") + context.add_user_message("Where is order 88213?") + + get "/activeagents/api/interactions", params: { minutes: [ 60, 120 ], limit: { n: 10 } } + + assert_response :success, response.body + assert_equal 1, JSON.parse(response.body)["interactions"].length + end + + test "session recordings coerce container-valued page, per_page, after_sequence and limit" do + recording = ActionAgent::SessionRecording.start_user_session!(page_url: "https://example.com/") + recording.record_action!(action_type: "click", selector: "button") + + get "/activeagents/api/session_recordings", params: { page: [ 1 ], per_page: { n: 20 } } + assert_response :success, response.body + assert_equal 1, JSON.parse(response.body).dig("pagination", "page") + + get "/activeagents/api/session_recordings/#{recording.id}/actions", params: { after_sequence: [ 0 ], limit: { n: 5 } } + assert_response :success, response.body + assert_equal 1, JSON.parse(response.body)["actions"].size + end + + test "sandbox compare rejects a providers value that is not a list of names" do + sandbox = ActionAgent::SandboxSession.create!(session_id: SecureRandom.uuid, status: :ready, expires_at: 1.hour.from_now) + + post "/activeagents/api/sandboxes/compare", params: { task: "Take a screenshot", providers: "anthropic", sandbox_id: sandbox.session_id } + assert_response :bad_request, response.body + + post "/activeagents/api/sandboxes/compare", params: { task: "Take a screenshot", providers: { a: "anthropic" }, sandbox_id: sandbox.session_id } + assert_response :bad_request, response.body + end +end diff --git a/actionagent/test/session_recording_privacy_test.rb b/actionagent/test/session_recording_privacy_test.rb index cfa96127..2e953b94 100644 --- a/actionagent/test/session_recording_privacy_test.rb +++ b/actionagent/test/session_recording_privacy_test.rb @@ -48,6 +48,29 @@ def setup assert_nil exported["metadata"]["password"] assert_not_includes response.body, "hunter2secret" end + + # The handoff state is a copy of the visitor's browser: cookies and web + # storage. Stripping only the top level of the metadata left the same + # secrets readable one key down, and as the top-level handoff_state key. + test "show strips cookies and web storage from the nested handoff state" do + @recording.update!(metadata: @recording.metadata.merge( + "handoff_state" => { + "url" => "https://example.com/checkout", + "cookies" => [ { "name" => "_session", "value" => "sekrit-cookie" } ], + "local_storage" => { "auth_token" => "lst-secret" }, + "session_storage" => { "csrf" => "sst-secret" } + } + )) + + get "/activeagents/api/session_recordings/#{@recording.id}" + + assert_response :success + body = JSON.parse(response.body)["recording"] + assert_equal "https://example.com/checkout", body.dig("handoff_state", "url") + assert_nil body.dig("handoff_state", "cookies") + assert_nil body.dig("metadata", "handoff_state", "cookies") + %w[sekrit-cookie lst-secret sst-secret].each { |secret| assert_not_includes response.body, secret } + end end # In a per-user install the list has to show the recordings the caller can diff --git a/actionagent/test/telemetry_correlation_test.rb b/actionagent/test/telemetry_correlation_test.rb index 6e0f2651..f2529b25 100644 --- a/actionagent/test/telemetry_correlation_test.rb +++ b/actionagent/test/telemetry_correlation_test.rb @@ -64,7 +64,7 @@ def setup end test "instrumented generations share the trace_id exposed in prompt_options" do - original_tracer = swap_global_tracer(ActiveAgent::Telemetry::Tracer.new(@configuration)) + previous = swap_global_tracer(ActiveAgent::Telemetry::Tracer.new(@configuration)) agent_class = Class.new(ApplicationAgent) do def self.name = "CorrelationProbeAgent" @@ -91,11 +91,11 @@ def ping assert_match(/\A\h{8}-\h{4}-\h{4}-\h{4}-\h{12}\z/, trace.trace_id, "trace_id should be the prompt_options UUID, not a tracer-generated hex id") ensure - swap_global_tracer(original_tracer) + restore_global_tracer(previous) end test "instrumented generations record provider and model attributes" do - original_tracer = swap_global_tracer(ActiveAgent::Telemetry::Tracer.new(@configuration)) + previous = swap_global_tracer(ActiveAgent::Telemetry::Tracer.new(@configuration)) agent_class = Class.new(ApplicationAgent) do def self.name = "AttributeProbeAgent" @@ -122,7 +122,7 @@ def ping assert_equal "mock-model", llm.dig("attributes", "llm.model") assert_operator prompt_span.dig("attributes", "messages.count").to_i, :>=, 1 ensure - swap_global_tracer(original_tracer) + restore_global_tracer(previous) end private @@ -144,13 +144,28 @@ def stored_payload } end + # Points the framework's global tracer at +tracer+ with local storage on, + # returning the state to hand back to +restore_global_tracer+. Left as it + # is, `local_storage` keeps `enabled?` true for the rest of the process, + # and every later generation in the suite is traced and registered as an + # observed agent. def swap_global_tracer(tracer) - previous = ActiveAgent::Telemetry.instance_variable_get(:@tracer) + config = ActiveAgent::Telemetry.configuration + previous = { + tracer: ActiveAgent::Telemetry.instance_variable_get(:@tracer), + enabled: config.enabled, + local_storage: config.local_storage + } ActiveAgent::Telemetry.instance_variable_set(:@tracer, tracer) - previous_enabled = ActiveAgent::Telemetry.configuration.enabled - ActiveAgent::Telemetry.configuration.enabled = true - ActiveAgent::Telemetry.configuration.local_storage = true - @restore_enabled = previous_enabled + config.enabled = true + config.local_storage = true previous end + + def restore_global_tracer(previous) + config = ActiveAgent::Telemetry.configuration + ActiveAgent::Telemetry.instance_variable_set(:@tracer, previous[:tracer]) + config.enabled = previous[:enabled] + config.local_storage = previous[:local_storage] + end end diff --git a/actionagent/test/telemetry_trace_test.rb b/actionagent/test/telemetry_trace_test.rb index 28aaa3d5..78060ea3 100644 --- a/actionagent/test/telemetry_trace_test.rb +++ b/actionagent/test/telemetry_trace_test.rb @@ -216,6 +216,26 @@ def prompt_span(roster) assert_equal 1, ActionAgent::Agent.where(agent_class_name: "SupportAgent").count end + # The case above passes with no owner association configured, where + # `for_owner(nil)` is `all`. A single-tenant host that DOES configure one — + # the ordinary Rails app, `owned_by :user` with multi_tenant off — resolves + # no owner per trace, and `for_owner(nil)` is then `none`: the dedupe lookup + # missed every time and each ingest registered another copy of one agent, + # with MAX_OBSERVED_PER_OWNER never engaging because the count it reads was + # always zero. Asserted on the registrar's own scoping seam, because the + # symptom depends on a host's owner model rather than on this dummy app's. + test "the dedupe scope is the whole table when there is no owner to resolve" do + trace = ActionAgent::TelemetryTrace.create_from_payload(payload(spans: [ root_span ])) + registrar = ActionAgent::AgentRegistrar.new(trace) + + scope = registrar.send(:agents_for_owner, nil) + + refute_equal "SELECT 1 AS one WHERE 1=0", scope.limit(1).to_sql, + "a single-tenant registrar must not dedupe against `none`" + assert_equal ActionAgent::Agent.count, scope.count, + "an unowned single-tenant ingest deduplicates against every agent row" + end + # Exercises AgentRegistrar's own rescue rather than a stub of it: a broken # registration must never cost a host app its telemetry. test "registration failure does not fail ingest" do diff --git a/docs/framework/dashboard.md b/docs/framework/dashboard.md index 053e47df..90acb55b 100644 --- a/docs/framework/dashboard.md +++ b/docs/framework/dashboard.md @@ -510,6 +510,21 @@ ActionAgent.configure do |config| end ``` +A browser that asks for a dashboard page without a valid session is sent +to `config.sign_in_path` when one is set — your app's sign-in page — and +otherwise shown a minimal session-expired page. API and MCP clients get a +bare 401 either way. `config.sign_out_path` is the endpoint the header's +"Sign out" item POSTs to (with `_method=delete` and the CSRF token); the +engine has no session of its own, so leave it unset to hide the item. + +```ruby +ActionAgent.configure do |config| + config.authentication_method = ->(controller) { controller.authenticate_admin! } + config.sign_in_path = "/admin/sign_in" + config.sign_out_path = "/admin/sign_out" +end +``` + Or constrain the mount in `config/routes.rb`: ```ruby diff --git a/docs/work/release-1.6.0/branch.md b/docs/work/release-1.6.0/branch.md new file mode 100644 index 00000000..7397f937 --- /dev/null +++ b/docs/work/release-1.6.0/branch.md @@ -0,0 +1,55 @@ +# Release 1.6.0 branch + +- Branch: `release/1.6.0` +- Base: main commit `51c7feb7` +- Tag: `v1.6.0` (not yet pushed) +- Ships: `activeagent` 1.6.0 and `actionagent` 1.6.0 from one tag + +## Commits + +| Commit | Purpose | +|--------|---------| +| _(this branch)_ | Version bumps, the dated 1.6.0 changelog heading, and these release notes | + +## Version jump + +`activeagent` and `actionagent` both go 1.5.2 -> 1.6.0. A minor, not a +patch: the cycle adds `ActiveAgent::Base#current_user` and the `as(...)` +caller seam, `SchemaTools.define`/`undefine` with a per-model registry, the +`active_agent:schema_tools` generator, schema tools served over the MCP +facade, caller inheritance through `delegate_to`, and the +`ungrounded_answer` evaluation fault. That is new public surface in both +gems. + +1.5.2 shipped the whole SchemaTools feature as a patch, so the repository +has precedent either way; 1.6.0 is the owner's call for this cycle, made +because seven new APIs is more surface than a patch number advertises. + +The engine's floor on the framework (`activeagent >= 1.4`) is unchanged and +still correct: 1.6.0 satisfies it. `activeagents-telemetry` stays at +`~> 0.1`; nothing in this cycle calls a 0.3-only API, and `~> 0.1` already +resolves the published 0.3.0. + +## Behaviour changes worth calling out + +- `tools_succeeded` is awarded only for a tool the scenario expected. Suites + that were scoring wrong-tool runs as partial successes will report lower. +- `actor:` is stripped from tool arguments and from `params[params][actor]`, + so the caller cannot be named by the model or by a client. + +## Artifacts + +Built to `pkg/` with `bundle exec rake build_all`, **not pushed**: + +- `pkg/activeagent-1.6.0.gem` (220 KB, 194 files) +- `pkg/actionagent-1.6.0.gem` (512 KB, 111 files) + +Publish `activeagent` first; `actionagent` depends on it. + +## Publishing + +Tag-driven publishing does not currently work in this repository — see +`validation.md`. Every `release.yml` run to date has failed at +`No trusted publisher configured for this workflow found on rubygems.org`, +which is why 1.5.1 and 1.5.2 have no tags yet are on RubyGems. The owner +publishes these two archives by hand. diff --git a/docs/work/release-1.6.0/validation.md b/docs/work/release-1.6.0/validation.md new file mode 100644 index 00000000..9aabe6e6 --- /dev/null +++ b/docs/work/release-1.6.0/validation.md @@ -0,0 +1,61 @@ +# Release 1.6.0 validation + +## Verification gate: CI on the base commit + +The release is cut from `main` at `51c7feb7` (the #451 merge). CI is green on +that exact commit — run `34736861631`: + +| Job | Result | +|---|---| +| `test (3.2, gemfiles/rails7.gemfile)` | 1978 runs, 6525 assertions, 0 failures, 0 errors, 22 skips | +| `test (3.3, gemfiles/rails8.gemfile)` | 1978 runs, 6525 assertions, 0 failures, 0 errors, 22 skips | +| `test (3.4, gemfiles/rails8.gemfile)` | 1978 runs, 6525 assertions, 0 failures, 0 errors, 22 skips | +| `test (3.4, gemfiles/railsmain.gemfile)` | 1978 runs, 6525 assertions, 0 failures, 0 errors, 22 skips | +| `Test API Gems (3.4)` | 8 runs, 18 assertions, 0 failures, 0 errors | +| `lint` | success | + +`bin/lint` locally: 542 files inspected, no offenses. + +`bundle exec rake build_all` succeeds and both archives pass the task's own +content assertions (`lib/active_agent.rb`; `lib/action_agent.rb`, +`config/routes.rb`, `app/assets/builds/action_agent.{js,css}`). + +## Local suite: 34 errors, all environmental + +A local run on Ruby 3.4.9 reports `1982 runs, 0 failures, 34 errors, 21 +skips` — four more runs and 34 more errors than CI on the same commit. None +is release content. Two causes: + +1. **Placeholder API keys.** `.env.test` holds placeholders (its own comment + says CI holds the real keys), so tests that reach a live provider fail + with `Incorrect API key provided: test-ope***-key`. This accounts for the + `integration_test.rb` errors and several in `ruby_llm_provider_test.rb`. + +2. **`gemfiles/rails8.gemfile.lock` is not committed.** `git ls-files + gemfiles/` lists the `.gemfile` files only. CI therefore resolves + `ruby_llm` fresh at install time, while a local checkout keeps whatever + its untracked lock pinned — here 1.16.0, whose `RubyLLM::Message` exposes + `tool_calls` and `input_tokens` as readers but no longer as writers. + `test/providers/ruby_llm/ruby_llm_provider_test.rb`'s `StubProvider` + assigns them (`msg.input_tokens = 10`, `msg.tool_calls = {...}`), so it + raises `NoMethodError` locally and passes in CI. Same commit, same + declared dependency, different resolution. + +Cause 2 is worth fixing on its own branch — either commit the lock or adapt +the stub to construct a `Message` with those values rather than assigning +them. It is a CI/local divergence that hides real breakage, but it is not a +1.6.0 blocker and no code in this release touches it. + +## Local toolchain note + +`.tool-versions` says `ruby latest`, which resolves to 4.0.6 here. Rails 8.1 +cannot boot on it — `bin/test` dies at `cannot load such file -- +active_storage/engine` before running anything. CI tests 3.2/3.3/3.4 only. +Verify locally with `mise exec ruby@3.4.9`. + +## Not verified + +No host application was installed against the built 1.6.0 archives (the +1.5.2 release did do this). The gems build, contain their entry points, and +CI is green on the base commit; an end-to-end install check is still worth +doing before pushing if the owner wants the same bar as last cycle. diff --git a/docs/work/schema-tools-follow-ups/milestones.md b/docs/work/schema-tools-follow-ups/milestones.md index dc8db565..d29bd5bf 100644 --- a/docs/work/schema-tools-follow-ups/milestones.md +++ b/docs/work/schema-tools-follow-ups/milestones.md @@ -10,4 +10,4 @@ | M6 | Runtime definitions with a registry; descendants leak closed (#447, #441) | ⏳ CI after the main merge | | M7 | Delegated runs inherit the caller (#448) | ⏳ stacked on #443 | | M8 | Schema tools over MCP (#449, #439) | ⏳ stacked on #443 | -| M9 | A 1.5.3 release carrying all of it | not started — the release is the owner's call | +| M9 | A release carrying all of it | ⏳ prepared as 1.6.0 (`release/1.6.0`), awaiting the owner's `gem push` | diff --git a/lib/active_agent/schema_tools.rb b/lib/active_agent/schema_tools.rb index 66c8978c..55bd6531 100644 --- a/lib/active_agent/schema_tools.rb +++ b/lib/active_agent/schema_tools.rb @@ -68,6 +68,21 @@ class SchemaTools # ask for 10_000; this is what stops that from becoming the prompt. MAX_LIMIT = 100 + # Column types a range comparison is offered for. Strings and booleans + # are deliberately absent: a lexical `>` on a name column answers a + # question nobody asked. + RANGE_FILTERABLE_TYPES = %i[date datetime time integer float decimal].freeze + + # The comparison operators a range filter may use, mapped to the Arel + # predicate that builds them. Names are the ones models reach for + # unprompted (`before`/`after` for dates, `lt`/`gte` for numbers), so a + # reasonable guess resolves instead of erroring. + RANGE_OPERATORS = { + "before" => :lt, "after" => :gt, + "lt" => :lt, "lte" => :lteq, "gt" => :gt, "gte" => :gteq, + "on_or_before" => :lteq, "on_or_after" => :gteq + }.freeze + # Raised when a tool call names a column outside the declared allowlists, # or is otherwise outside the declared boundary. class UnpermittedAttribute < ArgumentError; end @@ -300,8 +315,13 @@ def relation_for(actor) # Validates and normalizes a filter hash against the allowlist. # + # A filter value is normally matched for equality. A Hash value instead + # declares a range — `{ "before" => "2026-01-01" }`, `{ "gte" => 10 }` — + # and may carry two bounds at once to express a window. + # # @api private - # @raise [UnpermittedAttribute] if any key is not declared filterable + # @raise [UnpermittedAttribute] if any key is not declared filterable, + # or a range names an operator that does not exist def permitted_filters!(arguments) filters = arguments.each_with_object({}) do |(key, value), memo| next if value.nil? @@ -318,6 +338,54 @@ def permitted_filters!(arguments) filters end + # Splits filters into equality pairs and range predicates. + # + # Kept separate from {.permitted_filters!} because the two halves are + # applied differently: equality goes to `where(hash)`, ranges have to be + # built through Arel. + # + # @api private + # @return [Array(Hash, Array)] + def partition_filters!(filters) + equality = {} + ranges = [] + + filters.each do |column, value| + if value.is_a?(Hash) + ranges.concat(range_predicates!(column, value)) + else + equality[column] = value + end + end + + [ equality, ranges ] + end + + # Builds Arel predicates for one column's range hash. + # + # Rails silently turns `where(col: { "before" => x })` into `col = NULL`, + # which matches nothing and reports zero rather than failing — the worst + # outcome for an agent, which reads it as a truthful empty answer. So an + # unknown operator is rejected loudly here instead. + # + # @api private + # @raise [UnpermittedAttribute] on an unknown operator + def range_predicates!(column, value) + arel = @model.arel_table[column] + type = @model.type_for_attribute(column) + + value.map do |operator, operand| + predicate = RANGE_OPERATORS[operator.to_s] + unless predicate + raise UnpermittedAttribute, + "`#{operator}` is not a valid comparison for `#{column}`. " \ + "Allowed comparisons: #{RANGE_OPERATORS.keys.join(", ")}" + end + + arel.public_send(predicate, type.cast(operand)) + end + end + # Projects a record down to the declared return columns. # # The projection happens in SQL (+select+) as well as here, but the Ruby @@ -386,7 +454,40 @@ def filter_properties ) properties = schema[:schema][:properties] - filterable.index_with { |column| (properties[column] || { type: "string" }).deep_dup } + filterable.index_with do |column| + scalar = (properties[column] || { type: "string" }).deep_dup + range_filterable?(column) ? with_range_form(column, scalar) : scalar + end + end + + # Dates, times and numbers are the columns a question like "overdue" or + # "more than 10" actually needs a comparison on. + def range_filterable?(column) + RANGE_FILTERABLE_TYPES.include?(@model.type_for_attribute(column).type) + end + + # Offers a column as either a scalar (equality) or a range object. + # + # Without this the range form works but is undiscoverable: a model shown + # only `{type: "string", format: "date"}` has no way to know it may ask + # for `before`, and answers date questions with an equality match or no + # filter at all. + def with_range_form(column, scalar) + operand = scalar.slice(:type, :format) + description = scalar[:description] + + { + description: [ description, "Accepts an exact value, or a range object such as " \ + "{\"before\": ...} / {\"gte\": ...} (#{RANGE_OPERATORS.keys.join(", ")})." ].compact.join(" "), + anyOf: [ + scalar.except(:description), + { + type: "object", + properties: RANGE_OPERATORS.keys.index_with { operand.dup }, + additionalProperties: false + } + ] + } end def resource_name @@ -415,10 +516,10 @@ def define_find_tool ) define_singleton_method(name) do |actor: nil, limit: nil, **arguments| - filters = permitted_filters!(arguments) + equality, ranges = partition_filters!(permitted_filters!(arguments)) capped = normalize_limit(limit) - relation = relation_for(actor).where(filters) + relation = ranges.reduce(relation_for(actor).where(equality)) { |rel, p| rel.where(p) } # One extra row distinguishes "exactly at the limit" from "more than # the limit", without a second COUNT query. records = relation.limit(capped + 1).to_a @@ -443,9 +544,10 @@ def define_count_tool ) define_singleton_method(name) do |actor: nil, **arguments| - filters = permitted_filters!(arguments) + equality, ranges = partition_filters!(permitted_filters!(arguments)) + relation = ranges.reduce(relation_for(actor).where(equality)) { |rel, p| rel.where(p) } - { count: relation_for(actor).where(filters).count } + { count: relation.count } end end diff --git a/lib/active_agent/telemetry.rb b/lib/active_agent/telemetry.rb index 0a4d96ee..e6731143 100644 --- a/lib/active_agent/telemetry.rb +++ b/lib/active_agent/telemetry.rb @@ -78,9 +78,31 @@ def configuration # end def configure yield configuration if block_given? + install_instrumentation! if enabled? configuration end + # Installs generation instrumentation on ActiveAgent::Base. + # + # The railtie also does this at boot, but only for configuration already + # loaded by then (activeagent.yml, config.active_agent.telemetry). A host + # app that enables telemetry from its own initializer runs *after* + # railties, so that check has already seen `enabled? == false` and + # skipped the install — leaving telemetry enabled but nothing + # instrumented, and so no traces despite a valid local_store. Calling it + # from {.configure} as well makes the install order-independent; + # `instrument_telemetry!` is idempotent, so the two paths cannot + # double-prepend. + # + # @api private + # @return [void] + def install_instrumentation! + return unless defined?(ActiveAgent::Base) + + ActiveAgent::Base.include(Instrumentation) + ActiveAgent::Base.instrument_telemetry! + end + # Resets the configuration to defaults. # # @return [Configuration] New default configuration diff --git a/lib/active_agent/version.rb b/lib/active_agent/version.rb index e43f3d46..1965fc9a 100644 --- a/lib/active_agent/version.rb +++ b/lib/active_agent/version.rb @@ -1,3 +1,3 @@ module ActiveAgent - VERSION = "1.5.2" + VERSION = "1.6.2" end diff --git a/test/schema_tools_test.rb b/test/schema_tools_test.rb index aec2704c..b03b5b5e 100644 --- a/test/schema_tools_test.rb +++ b/test/schema_tools_test.rb @@ -453,4 +453,102 @@ def resolve = @actor ? @scope.all : @scope.none Class.new(ActiveAgent::SchemaTools) { scope_by_policy } end end + + # --- Range filters ----------------------------------------------------- + # + # Rails turns `where(col: {"before" => x})` into `col = NULL`, so before + # these were supported a range filter matched nothing and reported zero. + # An agent reads that as a truthful empty answer, which is why the silent + # case is tested as carefully as the working one. + + class DatedPostTools < ActiveAgent::SchemaTools + model Post + filterable :published_at, :published + returns :id, :title, :published_at + end + + test "range filter compares instead of matching nothing" do + Post.delete_all + old = Post.create!(title: "Old", content: "body", user: @alice, published_at: 10.days.ago) + Post.create!(title: "New", content: "body", user: @alice, published_at: 1.day.from_now) + + result = DatedPostTools.call("find_posts", published_at: { "before" => Time.current.iso8601 }) + + assert_equal 1, result[:count] + assert_equal [ old.title ], result[:results].map { |r| r[:title] } + end + + test "count applies a range filter" do + Post.delete_all + Post.create!(title: "Old", content: "body", user: @alice, published_at: 10.days.ago) + Post.create!(title: "New", content: "body", user: @alice, published_at: 1.day.from_now) + + assert_equal 1, DatedPostTools.call("count_posts", published_at: { "after" => Time.current.iso8601 })[:count] + end + + test "range filter accepts two bounds as a window" do + Post.delete_all + Post.create!(title: "Way old", content: "body", user: @alice, published_at: 30.days.ago) + inside = Post.create!(title: "Inside", content: "body", user: @alice, published_at: 5.days.ago) + Post.create!(title: "Future", content: "body", user: @alice, published_at: 5.days.from_now) + + result = DatedPostTools.call( + "find_posts", + published_at: { "after" => 10.days.ago.iso8601, "before" => Time.current.iso8601 } + ) + + assert_equal 1, result[:count] + assert_equal [ inside.title ], result[:results].map { |r| r[:title] } + end + + test "range filter combines with an equality filter" do + Post.delete_all + Post.create!(title: "Old published", content: "body", user: @alice, published_at: 10.days.ago, published: true) + Post.create!(title: "Old draft", content: "body", user: @alice, published_at: 10.days.ago, published: false) + + result = DatedPostTools.call( + "find_posts", published: true, published_at: { "before" => Time.current.iso8601 } + ) + + assert_equal 1, result[:count] + assert_equal [ "Old published" ], result[:results].map { |r| r[:title] } + end + + test "rejects an unknown comparison rather than reporting zero" do + Post.delete_all + Post.create!(title: "Old", content: "body", user: @alice, published_at: 10.days.ago) + + result = DatedPostTools.call("find_posts", published_at: { "roughly_before" => Time.current.iso8601 }) + + assert_match(/not a valid comparison/, result[:error]) + refute result.key?(:results), "must not return records when a comparison is rejected" + end + + test "a rejected comparison never silently widens or narrows the answer" do + Post.delete_all + Post.create!(title: "Old", content: "body", user: @alice, published_at: 10.days.ago) + + # The failure this guards: returning {count: 0} (silently narrowed) or the + # unfiltered set (silently widened) instead of an error. + result = DatedPostTools.call("count_posts", published_at: { "bogus" => "2026-01-01" }) + + assert result.key?(:error) + refute result.key?(:count) + end + + test "range filters are offered on comparable columns only" do + properties = DatedPostTools.tool_definitions.find { |d| d[:name] == "find_posts" } + .dig(:parameters, :properties) + + assert properties[:published_at].key?(:anyOf), "a datetime column must offer the range form" + refute properties[:published].key?(:anyOf), "a boolean column must not offer a range form" + end + + test "the advertised range operators are the ones accepted" do + properties = DatedPostTools.tool_definitions.find { |d| d[:name] == "find_posts" } + .dig(:parameters, :properties) + advertised = properties[:published_at][:anyOf].last[:properties].keys.map(&:to_s) + + assert_equal ActiveAgent::SchemaTools::RANGE_OPERATORS.keys.sort, advertised.sort + end end diff --git a/test/telemetry/instrumentation_install_test.rb b/test/telemetry/instrumentation_install_test.rb new file mode 100644 index 00000000..d734feea --- /dev/null +++ b/test/telemetry/instrumentation_install_test.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require "test_helper" + +# The railtie installs generation instrumentation at boot, but it can only +# see configuration that is already loaded by then. A host app that enables +# telemetry from its own `config/initializers/*.rb` runs *after* railties, so +# that check had already seen `enabled? == false` and skipped the install — +# leaving telemetry enabled, a valid local_store, and no traces at all. +# +# Configure is the one funnel every such app goes through, so the install +# happens there too and initializer order stops mattering. +class InstrumentationInstallTest < ActiveSupport::TestCase + setup do + @original = ActiveAgent::Telemetry.configuration + ActiveAgent::Telemetry.reset_configuration! + end + + teardown do + ActiveAgent::Telemetry.instance_variable_set(:@configuration, @original) + end + + def instrumented? + ActiveAgent::Base.ancestors.any? { |m| m.name.to_s.include?("GenerationInstrumentation") } + end + + test "enabling telemetry after boot installs instrumentation" do + ActiveAgent::Telemetry.configure do |config| + config.enabled = true + config.local_storage = true + end + + assert ActiveAgent::Telemetry.enabled?, "precondition: telemetry is enabled" + assert instrumented?, "enabling telemetry must install generation instrumentation" + end + + test "repeated configure calls do not prepend instrumentation twice" do + 2.times do + ActiveAgent::Telemetry.configure do |config| + config.enabled = true + config.local_storage = true + end + end + + matches = ActiveAgent::Base.ancestors.count { |m| m.name.to_s.include?("GenerationInstrumentation") } + assert_equal 1, matches, "instrument_telemetry! must stay idempotent across configure calls" + end + + test "configuring without enabling does not instrument" do + # Guards the inverse mistake: installing unconditionally would trace for + # apps that deliberately left telemetry off. + ActiveAgent::Telemetry.reset_configuration! + before = instrumented? + + ActiveAgent::Telemetry.configure { |config| config.sample_rate = 0.5 } + + assert_equal before, instrumented?, "a disabled configure must not install instrumentation" + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index c32ba385..5745b6a8 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -17,6 +17,15 @@ require "webmock/minitest" require "minitest/mock" +# Action Cable reads config/cable.yml for the current environment the first +# time its server class loads, and keeps what it read. With eager loading off +# that first time is whichever test first broadcasts or renders the dashboard, +# and a test that stubs Rails.env to a name with no section in cable.yml +# (engine_integration_test's staging case) must not be it: the config would +# come back empty, the adapter would fall back to redis, and every later +# broadcast in the process would raise. Load it here, under the test env. +ActionCable.server.config.cable + # Extract full path and relative path from caller_info def extract_path_info(caller_info) if caller_info =~ /(.+):(\d+):in/