Port local mainline: artifact generation, observability/security/config subsystems, and suite repairs#7
Open
codenamev wants to merge 18 commits into
Open
Port local mainline: artifact generation, observability/security/config subsystems, and suite repairs#7codenamev wants to merge 18 commits into
codenamev wants to merge 18 commits into
Conversation
Brings over the subsystems built on the local line: observability
(engine, dispatcher, pipeline, adapters, event context), security
(sanitizer, secure errors), configuration schemas, performance caching,
human intervention portal, CLI streaming/progress UI, the artifact
system foundations (Artifact, ArtifactGraph, Workspace, file_generation
capability), learning-system extensions, and their specs.
Conflicts with the remote line resolved in remote's favor structurally:
zeitwerk-only loading (no require_relative siblings in ported entry
points; human_intervention/{auth,monitoring,workflow}.rb renamed to
match their constants), remote's thread-safe assembly initialization,
LlmClient rate limiter combined with local streaming callbacks,
TaskFailure retryability combined with local sanitization, and Task
gains payload (remote) alongside workspace (local). The runtime config
class keeps the LegacyConfiguration name so the Configuration schema
module keeps its namespace. Dropped the unused oj dependency.
Async do at top level runs its reactor synchronously, so start never returned while the worker loops ran; every caller deadlocked. Workers now run on background threads with interruptible sleeps, stop joins them and drains queued batches, and the batch queues use Thread::Queue (Async::Queue#dequeue accepts no timeout). Also rewrites the racy concurrent-buffer spec that relied on dropped pushes completing.
Tempfile#path returns nil after unlink, so the rotated-file cleanup
glob "#{log_path}.*" evaluated to ".*" whenever an example never
touched the log before the after hook ran, deleting every dotfile in
the working directory (.gitignore, .rspec, .rubocop.yml, ...).
Capture the path before unlink and skip cleanup when it is nil.
Thor calls exit(1) on a missing required option; RSpec aborts the run mid-suite while still printing a normal-looking summary, so a random seed-dependent chunk of the suite silently never ran. An around hook now converts SystemExit into an ordinary failure. Also fixes agent create, which built agents through Agent.new with a block that Agent.new ignores (role and purpose were never set), and updates its spec to pass the now-required --purpose option.
Eviction ran at a fraction of max_size and always removed at least 10 entries, so a max_size 3 cache was emptied on its third set. Evict only when at capacity (memory threshold unchanged) and remove just the overflow. Also replaces the have(n) matcher, which needs the absent rspec-collection_matchers gem.
When the transport delivers no SSE chunks for a streamed request (some providers, and any webmock/VCR replay), streaming accumulated nothing and the client raised "Empty content returned from LLM". Retry once without streaming instead. The plan cassette duplicates each recorded interaction so the streamed attempt and its fallback both replay.
The security sanitizer stringifies hash keys, so failure.context[:key] silently returned nil whenever PII detection was enabled.
Description inference only matched literal underscore names, so "Analyze data and generate a report" never inferred data_analysis. Capability words now match description words on shared five-character stems (analyze/analysis, generate/generation). Explicit code-generation phrasing keeps its high importance even when the keyword scan already registered the capability. Also fixes the integration spec fixture that built its agent with Agent.new, whose block is ignored.
Gives error-driven verification failures a machine-readable context (:error_type, :timestamp) per the v0.3.0 error-handling standardization, populated by the LLM strategy's error path.
Aligns the event dispatcher, event context, and file observer with the consolidated v0.3.0 interfaces and updates their specs, which had drifted (removed FalconStreamBackend references, stale payload shapes, timestamp-ordering assumptions in FileAdapter#events_since).
Sanitizer pattern and key handling fixes plus updated expectations, and removes a duplicated respond_to? guard in SecureErrorMixin that also dropped retryable: false from secure hashes.
Schema constraint and builder fixes with updated specs; float default comparisons now use an epsilon and range checks use Comparable#between?.
Observable spec helper classes are now anonymous (a same-named top-level class in another spec file merged into them and broke whichever loaded second). Replaces positional ObservabilityEngine#notify calls with the keyword interface, the unsupported expect_any_instance_of + have_received combination with an argument capture, a reference to the removed UI::Dashboard, and stubs the LLM verification error path directly since the strategy is a stub that never calls the client.
Stubs retry backoff in the LLM client spec (8s -> 0.1s), makes the human-intervention monitoring loop's sleep interruptible so stop! no longer burns its join timeout (20s -> 2s in the portal integration spec), shrinks a simulated-timeout sleep, tags the performance benchmarks :slow (run with RUN_SLOW_TESTS=1) while repairing their stale engine APIs, disallows real HTTP when no cassette is loaded, and sets the coverage gate to the current 74% floor to block regressions.
ArtifactGenerator drives the file_generation capability against a task's workspace and returns an ArtifactGenerationResult. Tasks gain an artifact_mode flag and requires_artifacts?, and Agent#execute routes artifact tasks through the generator. Capability registration moves from load time into register_standard_capabilities so the logger exists first; agent errors propagate unwrapped for accurate failure reporting, and SecurityError from workspace validation surfaces as a failure result. Includes the artifact-system design docs.
The CLI writes result-<timestamp>.json into the current directory on every plan execution, so test runs kept littering the repo root.
The gem no longer defines AuthenticationError, so guard it like the other legacy constants in map_openai_error, drop the rescue of the removed Oj parser error, and have spec_helper load the openai gem before shimming its namespace (zeitwerk defers LlmClient, so nothing else loads it at spec-boot time). Adds AuthenticationError to the shims.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR rebases the local mainline (57 commits that diverged from
mainat 50d8e4c) onto currentmain. It lands as one ported base commit plus 16 scoped commits, all verified against the full suite.What it adds
ArtifactGenerator+ArtifactGenerationResultdrive thefile_generationcapability against a task'sWorkspace(with path-traversal and extension security validation);Taskgainsworkspace/artifact_mode, andAgent#executeroutes artifact tasks through the generator. Design docs underdocs/future/artifact-system/.EventPipeline#startdeadlock (synchronousAsyncblock → background threads), a spec cleanup hook that deleted repo dotfiles via a nil-Tempfile#pathglob, a ThorSystemExitsilently truncating random suite runs (now converted to a failure by spec_helper), cache LRU eviction wiping small caches, LLM streaming empty-content fallback, and ~100 stale spec expectations.Merge decisions (remote structure wins)
require_relativechains, andhuman_intervention/{auth,monitoring,workflow}.rbwere renamed to match their constants.LlmClient: main's rate limiter combined with local streaming callbacks;map_openai_errornow guardsOpenAI::AuthenticationErrorlike the other legacy constants (current ruby-openai doesn't define it); dropped the unusedojdependency.TaskFailure: main's retryability verdict combined with local sanitization (context keys normalized to symbols).Task: main'spayload/dependency_outputsalongside localworkspace/artifact_mode.LegacyConfigurationname so theAgentic::Configurationschema module keeps its namespace.Agent.buildin the CLI) dropped where main had already fixed them independently.Notes for reviewers
:slow(run withRUN_SLOW_TESTS=1).Test plan
bundle exec rake— 1375 examples, 0 failures; StandardRB cleanZeitwerk::Loader.eager_load_allloads every constant