diff --git a/CHANGELOG.md b/CHANGELOG.md index a314b10..08bae61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ ## [Unreleased] ### Fixed +- `require "agentic"` no longer depends on the dev bundle: `ArtifactGraph` runs on the standard library's `TSort` instead of rgl (which was never a declared dependency), the dead `require "base64"` in `AuthenticationSystem` is gone (a LoadError on Ruby >= 3.4 under Bundler), and `concurrent-ruby` and `tsort` are declared as the runtime dependencies they are. A spec now checks every `require` under lib/ against the gemspec +- `ArtifactGraph#detect_cycles` returns the artifacts that actually form each cycle instead of listing every artifact in the graph - Test suite loads on Ruby 4.0: `benchmark` is a bundled (not default) gem there, so it is now declared in the Gemfile - `EventDispatcher` slow-observer spec uses a monotonic clock and a looser bound so it no longer flakes on loaded hosts diff --git a/Gemfile b/Gemfile index 6408a41..19a1636 100644 --- a/Gemfile +++ b/Gemfile @@ -24,4 +24,3 @@ gem "timecop" gem "simplecov", require: false gem "memory_profiler" gem "benchmark" # bundled gem since Ruby 3.5; not loadable by default on 4.0 -gem "rgl", "~> 0.6" diff --git a/Gemfile.lock b/Gemfile.lock index a9dc9f1..0c18988 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -4,12 +4,14 @@ PATH agentic (0.2.0) async (~> 2.0) cgi + concurrent-ruby (~> 1.0) dry-schema logger ostruct pastel (~> 0.8) ruby-openai thor (~> 1.2) + tsort tty-box (~> 0.7) tty-cursor (~> 0.7) tty-progressbar (~> 0.18) @@ -95,7 +97,6 @@ GEM net-http (0.4.1) uri ostruct (0.6.1) - pairing_heap (3.1.1) parallel (1.24.0) parser (3.2.2.4) ast (~> 2.4.1) @@ -108,10 +109,6 @@ GEM rake (13.2.1) regexp_parser (2.8.3) rexml (3.4.4) - rgl (0.6.7) - pairing_heap (>= 0.3, < 4.0) - rexml (~> 3.2, >= 3.2.4) - stream (~> 0.5.3) rspec (3.13.0) rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) @@ -159,7 +156,6 @@ GEM standard-performance (1.2.1) lint_roller (~> 1.1) rubocop-performance (~> 1.19.1) - stream (0.5.6) strings (0.2.1) strings-ansi (~> 0.2) unicode-display_width (>= 1.5, < 3.0) @@ -168,6 +164,7 @@ GEM thor (1.3.2) timecop (0.9.10) traces (0.15.2) + tsort (0.2.0) tty-box (0.7.0) pastel (~> 0.8) strings (~> 0.2.0) @@ -210,7 +207,6 @@ DEPENDENCIES io-event (<= 1.11.2) memory_profiler rake (~> 13.0) - rgl (~> 0.6) rspec (~> 3.0) simplecov standard (~> 1.3) diff --git a/agentic.gemspec b/agentic.gemspec index 026659e..a584e85 100644 --- a/agentic.gemspec +++ b/agentic.gemspec @@ -48,6 +48,8 @@ Gem::Specification.new do |spec| spec.add_dependency "ostruct" spec.add_dependency "logger" # bundled gem as of Ruby 3.5 - declare what you require spec.add_dependency "cgi" # trimmed to a bundled gem in Ruby 3.5; used for CGI.escape + spec.add_dependency "concurrent-ruby", "~> 1.0" # Observability::EventPipeline's thread pool; only reached us through dry-core, which asks the same + spec.add_dependency "tsort" # bundled gem as of Ruby 4.1; ArtifactGraph's sort and cycle detection # For more information and examples about making a new gem, check out our # guide at: https://bundler.io/guides/creating_gem.html diff --git a/lib/agentic/artifact_graph.rb b/lib/agentic/artifact_graph.rb index ae48cd9..67b0c34 100644 --- a/lib/agentic/artifact_graph.rb +++ b/lib/agentic/artifact_graph.rb @@ -1,11 +1,9 @@ # frozen_string_literal: true -require "rgl/adjacency" -require "rgl/traversal" -require "rgl/topsort" +require "tsort" module Agentic - # Manages graph of artifact relationships using RGL (Ruby Graph Library) + # Manages the graph of artifact relationships # # ArtifactGraph maintains a directed graph where nodes are artifacts and # edges represent "references" relationships. For example, if UserService.rb @@ -17,6 +15,9 @@ module Agentic # - Circular dependency detection # - Topological sorting (build order) # + # Storage is a Hash from artifact name to the names it references; the + # standard library's TSort does the sorting and the cycle detection. + # # @example Building a graph # graph = ArtifactGraph.new # graph.add_node(user_artifact) @@ -32,8 +33,8 @@ class ArtifactGraph include Enumerable def initialize - @graph = RGL::DirectedAdjacencyGraph.new @artifacts = {} # artifact_name => Artifact object + @references = {} # artifact_name => [names it references] end # Add an artifact node to the graph @@ -58,21 +59,17 @@ def initialize # graph.add_node(artifact) def add_node(artifact) @artifacts[artifact.name] = artifact - @graph.add_vertex(artifact.name) + references = (@references[artifact.name] ||= []) - # Add edges for each reference: ref -> artifact - # (This means ref must come before artifact in topological sort) artifact.references.each do |ref_name| - # Ensure referenced vertex exists (even if artifact not added yet) - @graph.add_vertex(ref_name) unless @graph.has_vertex?(ref_name) - @graph.add_edge(ref_name, artifact.name) + # Ensure the referenced vertex exists (even if that artifact isn't added yet) + @references[ref_name] ||= [] + references << ref_name unless references.include?(ref_name) end end # Get artifacts that the given artifact depends on (direct dependencies) # - # With edges as ref -> artifact, dependencies are incoming edges. - # # @param artifact [Artifact, String] Artifact object or artifact name # @return [Array] Artifacts this one directly references # @@ -80,19 +77,14 @@ def add_node(artifact) # service = graph.find_node(name: "service.rb") # deps = graph.dependencies_of(service) # => [user_artifact] def dependencies_of(artifact) - artifact_name = artifact.is_a?(String) ? artifact : artifact.name - return [] unless @graph.has_vertex?(artifact_name) + artifact_name = name_of(artifact) + return [] unless @references.key?(artifact_name) - # Find vertices that have edges pointing TO this artifact (incoming edges) - @graph.vertices.select { |v| @graph.has_edge?(v, artifact_name) } - .map { |name| @artifacts[name] } - .compact + @references[artifact_name].map { |name| @artifacts[name] }.compact end # Get artifacts that depend on the given artifact (reverse dependencies) # - # With edges as ref -> artifact, dependents are outgoing edges. - # # @param artifact [Artifact, String] Artifact object or artifact name # @return [Array] Artifacts that reference this one # @@ -100,17 +92,18 @@ def dependencies_of(artifact) # user = graph.find_node(name: "user.rb") # dependents = graph.dependents_of(user) # => [service_artifact] def dependents_of(artifact) - artifact_name = artifact.is_a?(String) ? artifact : artifact.name - return [] unless @graph.has_vertex?(artifact_name) + artifact_name = name_of(artifact) + return [] unless @references.key?(artifact_name) - # Find vertices that this artifact has edges pointing TO (outgoing edges) - @graph.adjacent_vertices(artifact_name).map { |name| @artifacts[name] }.compact + @references.filter_map do |name, references| + @artifacts[name] if references.include?(artifact_name) + end end # Detect circular dependencies in the graph # - # Attempts to perform topological sort - if it fails or returns fewer vertices - # than the graph contains, cycles exist. + # Each strongly connected component with more than one artifact, or a + # single artifact that references itself, is a cycle. # # @return [Array>] Arrays of artifact names forming cycles # @@ -122,19 +115,7 @@ def dependents_of(artifact) # end # end def detect_cycles - # Try topsort - sorted = @graph.topsort_iterator.to_a - - # If sorted result has fewer vertices than the graph, there's a cycle - if sorted.size < @graph.vertices.size - # Return all vertices as a single cycle (exact cycle determination is complex) - [@graph.vertices.to_a] - else - [] # No cycles - end - rescue - # If topsort fails for any reason, assume cycle - [@graph.vertices.to_a] + strongly_connected_components.select { |component| cycle?(component) } end # Check if graph has circular dependencies @@ -156,14 +137,11 @@ def has_cycles? # sorted = graph.topological_sort # sorted.each { |a| puts "Generate: #{a.name}" } def topological_sort - # Check for cycles first - cycles = detect_cycles - if cycles.any? - raise CircularDependencyError, "Circular dependency detected: #{cycles.first.join(" -> ")}" - end + strongly_connected_components.map do |component| + raise CircularDependencyError, "Circular dependency detected: #{component.join(" -> ")}" if cycle?(component) - sorted_names = @graph.topsort_iterator.to_a - sorted_names.map { |name| @artifacts[name] }.compact + @artifacts[component.first] + end.compact end # Find artifact by name and optionally type @@ -220,7 +198,7 @@ def each(&block) # # @return [String] Human-readable graph description def to_s - "" + "" end # Inspection string for debugging @@ -230,7 +208,39 @@ def inspect artifacts_summary = @artifacts.keys.first(5).join(", ") artifacts_summary += ", ..." if @artifacts.size > 5 - "#" + "#" + end + + private + + def name_of(artifact) + artifact.is_a?(String) ? artifact : artifact.name + end + + def edge_count + @references.values.sum(&:size) + end + + # Tarjan's components in dependency order: this is exactly how TSort#tsort + # is built, minus the exception, so one pass serves both sorting and + # cycle detection. TSort treats a self-loop as a non-cycle; we don't. + def strongly_connected_components + TSort.strongly_connected_components(method(:each_name), method(:each_reference)) + end + + def cycle?(component) + component.size > 1 || @references[component.first].include?(component.first) + end + + # TSort's node enumerator: every vertex, added or merely referenced + def each_name(&block) + @references.each_key(&block) + end + + # TSort's child enumerator: an artifact's children are the artifacts it + # references, so the sort yields dependencies before dependents + def each_reference(name, &block) + @references.fetch(name, []).each(&block) end end diff --git a/lib/agentic/human_intervention/authentication_system.rb b/lib/agentic/human_intervention/authentication_system.rb index 9c8db7c..cb2b9d3 100644 --- a/lib/agentic/human_intervention/authentication_system.rb +++ b/lib/agentic/human_intervention/authentication_system.rb @@ -2,7 +2,6 @@ require "digest" require "securerandom" -require "base64" require "openssl" require "json" diff --git a/spec/agentic/artifact_graph_spec.rb b/spec/agentic/artifact_graph_spec.rb index 7f8261b..3e101e7 100644 --- a/spec/agentic/artifact_graph_spec.rb +++ b/spec/agentic/artifact_graph_spec.rb @@ -221,6 +221,29 @@ expect(cycles).not_to be_empty expect(cycles.first).to match_array(["a.rb", "b.rb", "c.rb"]) end + + it "reports only the artifacts that form the cycle" do + graph = described_class.new + + a = Agentic::Artifact.new(name: "a.rb", type: :ruby_class, content: "", references: ["b.rb"]) + b = Agentic::Artifact.new(name: "b.rb", type: :ruby_class, content: "", references: ["a.rb"]) + bystander = Agentic::Artifact.new(name: "c.rb", type: :ruby_class, content: "", references: ["a.rb"]) + + graph.add_node(a) + graph.add_node(b) + graph.add_node(bystander) + + expect(graph.detect_cycles).to contain_exactly(match_array(["a.rb", "b.rb"])) + end + + it "treats a self-reference as a cycle" do + graph = described_class.new + graph.add_node(Agentic::Artifact.new(name: "a.rb", type: :ruby_class, content: "", references: ["a.rb"])) + graph.add_node(user_artifact) + + expect(graph.detect_cycles).to eq([["a.rb"]]) + expect { graph.topological_sort }.to raise_error(Agentic::CircularDependencyError, /a\.rb/) + end end describe "#has_cycles?" do diff --git a/spec/lib_requires_spec.rb b/spec/lib_requires_spec.rb new file mode 100644 index 0000000..1330937 --- /dev/null +++ b/spec/lib_requires_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require "rubygems" + +# Every `require` under lib/ must resolve to something a consumer gets from +# `gem install agentic`: Ruby's own library, or a gem the gemspec declares as +# a runtime dependency. The dev bundle hides the gap (rgl sat in the Gemfile, +# base64 rode in on another gem's dependency), so CI stayed green while +# `require "agentic"; Agentic::ArtifactGraph` raised LoadError for consumers. +# Default gems that Ruby has promoted, or announced it will promote, to +# bundled gems count as needing a declaration: that is how logger and cgi are +# already handled. +RSpec.describe "lib/ requires" do + let(:gemspec) { Gem::Specification.load(File.expand_path("../agentic.gemspec", __dir__)) } + let(:declared) { gemspec.runtime_dependencies.map(&:name) } + let(:lib_dir) { File.expand_path("../lib", __dir__) } + + # Gem::BUNDLED_GEMS::SINCE is Ruby's own table (3.3+). The literal list is + # 4.0's table, so CI on an older interpreter applies the same verdicts. + let(:bundled) do + announced = defined?(Gem::BUNDLED_GEMS::SINCE) ? Gem::BUNDLED_GEMS::SINCE.keys : [] + known = %w[abbrev base64 benchmark bigdecimal csv drb fiddle getoptlong irb logger mutex_m nkf observer + ostruct pstore racc rdoc reline resolv-replace rinda syslog tsort win32ole] + (announced + known).uniq + end + + # [require path, lib-relative file] for every require of something outside the gem + let(:requires) do + Dir[File.join(lib_dir, "**", "*.rb")].sort.flat_map do |file| + File.foreach(file).filter_map do |line| + path = line[/\A\s*require\s+["']([^"']+)["']/, 1] + [path, file.delete_prefix("#{lib_dir}/")] if path && !path.start_with?("agentic") + end + end + end + + # A file in the stdlib directories, or a feature the interpreter provides + # without one (Set is built in on Ruby 4.0, so "set.rb" sits bare in + # $LOADED_FEATURES next to "enumerator.so"). + def shipped_with_ruby?(path) + return true if $LOADED_FEATURES.include?("#{path}.rb") || $LOADED_FEATURES.include?("#{path}.so") + + %w[rubylibdir archdir].any? do |dir| + ["rb", RbConfig::CONFIG["DLEXT"]].any? { |ext| File.exist?(File.join(RbConfig::CONFIG[dir], "#{path}.#{ext}")) } + end + end + + it "sees the requires it guards" do + expect(requires.map(&:first)).to include("json", "zeitwerk", "tsort", "concurrent") + end + + it "only requires Ruby's library or declared runtime dependencies" do + undeclared = requires.uniq.filter_map do |path, file| + owner = Gem::Specification.find_by_path(path) + name = owner ? owner.name : path.split("/").first + next if declared.include?(name) + next if (owner&.default_gem? || shipped_with_ruby?(path)) && !bundled.include?(name) + + "#{file}: require \"#{path}\" (#{name})" + end + + expect(undeclared).to be_empty, "requires nothing declares as a runtime dependency:\n #{undeclared.uniq.join("\n ")}" + end +end