Repository navigation
PlanOrchestratorConfig has no reader: nothing accepts it, its spec tests the orchestrator, and its defaults describe an orchestrator that does not exist #69
Description
Hypothesis
PlanOrchestratorConfig is a leftover from the first orchestrator design that nothing constructs, nothing accepts, and nothing tests, and its defaults describe an orchestrator that does not exist. A reader who finds it in the gem builds PlanOrchestratorConfig.new(continue_on_failure: false) expecting fail-fast and gets neither fail-fast nor an error, because there is no way to hand the object to anything.
Evidence
lib/agentic/plan_orchestrator_config.rb(57 lines, added 2025-05-20, shipped in 0.2.0). Zero references inlib/,exe/, README,docs/.grep -rn PlanOrchestratorConfig lib exe README.md docsreturns only the file itself.PlanOrchestrator.newtakesplan_id:, concurrency_limit:, retry_policy:, lifecycle_hooks:as plain keywords (plan_orchestrator.rb:24). There is noconfig:parameter and no.from_config. The CLI builds the orchestrator with keywords (cli.rb:656).spec/agentic/plan_orchestrator_config_spec.rblooks like coverage and is not: it isRSpec.describe Agentic::PlanOrchestrator, "configuration"and tests the orchestrator'sretry_policy:keyword. The config class has no spec.- The defaults lie about the runtime.
continue_on_failure: trueis the default, but the orchestrator's real failure policy is skip-the-dependents (Add :skipped terminal state for dependents of failed tasks #34), and Per-task failure tolerance: on_failure: continue (#23 C2) #40 adds the actual per-task knob ason_failure: :continue.async: trueis the default and there is no synchronous path:execute_planalways runs underAsync.to_hemits aretry_configshape (max_retries/backoff_strategy/backoff_options) that omitsretryable_errors, which is the key the orchestrator'sretry_policy:actually branches on. - GitHub code search for
Agentic::PlanOrchestratorConfigoutside this repo: no hits.
Same shape as #44, #46, #60: a declaration with no reader. This one is worse than dead code because it is dead code with a contract, and the contract contradicts the live one.
Experiment that would validate it
Write the test the class has never had: construct the config, pass it to the orchestrator, assert the behaviour the field names promise. It cannot be written, which is the finding. The remaining question is only "wire or remove", and the measurement is the GitHub search above (zero consumers) plus a note in the next release's CHANGELOG to catch anyone we missed.
Cost
- Remove: delete the file, add a
DeprecatedCHANGELOG line, done. Semver says it is public API because 0.2.0 shipped it, so the honest path is a one-minor deprecation shim (warnininitialize) and removal in the following minor. - Wire: add
config:toPlanOrchestrator.new, makecontinue_on_failureset the defaulton_failure:policy foradd_taskonce Per-task failure tolerance: on_failure: continue (#23 C2) #40 lands, and deleteasyncbecause it cannot mean anything. That is a second knob for the same thing Per-task failure tolerance: on_failure: continue (#23 C2) #40 already gives per task.
Class: public-api-change (L0)
Minerva's lean: Deprecate now and remove next minor; #40's per-task on_failure: is the right shape for failure tolerance, and a plan-level boolean on top would be the speculative abstraction the CONSTITUTION forbids.
Labels: loop:self, status:analyzed
Origin: loop:self session 2026-10-15, declared-but-never-read audit. Checked closed and deferred issues: nothing on this class. Related: #34 (skip state), #40 (per-task on_failure), #60 (same audit shape).
Activity